[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Libvir] Supporting new Xen 3.0.3 blktap backend for file devices



On Fri, Oct 06, 2006 at 04:44:17AM -0400, Daniel Veillard wrote:
> On Thu, Oct 05, 2006 at 07:23:15PM +0100, Daniel P. Berrange wrote:
> > diff -u -r1.66 xend_internal.c
> > --- src/xend_internal.c	29 Sep 2006 12:00:58 -0000	1.66
> > +++ src/xend_internal.c	5 Oct 2006 18:12:33 -0000
> > @@ -1499,7 +1499,7 @@
> >  	for (i = 0, j = 0;(i < 32) && (tmp[j] != 0);j++) {
> >  	    if (((tmp[j] >= '0') && (tmp[j] <= '9')) ||
> >  	        ((tmp[j] >= 'a') && (tmp[j] <= 'f')))
> > -		compact[i++] = tmp[j];
> > +            compact[i++] = tmp[j];
> 
>   maybe we should just add the full set of { } for the innner constructs 
> too if reformatting.

Will do.

> >  	    else if ((tmp[j] >= 'A') && (tmp[j] <= 'F'))
> >  	        compact[i++] = tmp[j] + 'a' - 'A';
> >  	}
> > @@ -1546,95 +1546,116 @@
> > +            drvName = malloc((offset-src)+1);
> 
>   I agree that if we OOM there it's gonna be messy anyway but let's catch
> NULL returns on allocs as much as possible

Don't know what I was thinking. Clearly I should be checking malloc() calls
for failure. Will fix.

> > +            strncpy(drvName, src, (offset-src));
> > +            drvName[offset-src] = '\0';
> > +
> > +            src = offset + 1;
> > +
> > +            if (!strcmp(drvName, "tap")) {
> > +                offset = strchr(src, ':');
> > +                if (!offset)
> > +                    goto bad_parse;
> > +
> > +                drvType = malloc((offset-src)+1);
> 
>   Same here. If failing a libvirt error and going to bad_parse should be
> sufficient I guess.

Will fix.

> 
> > +                    } else if (!strcmp(offset, ":disk")) {
> > +                        /* defualt anyway */
> 
>   /* default */ :-)
> > +            } else if ((drvName == NULL) &&
> > +                       (xmlStrEqual(cur->name, BAD_CAST "driver"))) {
> > +                drvName = xmlGetProp(cur, BAD_CAST "name");
> 
>   testing for NULL would be good, if the attribute is missing we should
> not crash

Yes, its fine for this to be NULL - i guess I was just lucky that this
next strcmp didn't crash when passed NULL. Will fix.

> > +                if (!strcmp((const char *)drvName, "tap"))
> > +                    drvType = xmlGetProp(cur, BAD_CAST "type");
> >              } else if (xmlStrEqual(cur->name, BAD_CAST "readonly")) {
> >                  ro = 1;
> >              }

Regards,
Dan.
-- 
|=- Red Hat, Engineering, Emerging Technologies, Boston.  +1 978 392 2496 -=|
|=-           Perl modules: http://search.cpan.org/~danberr/              -=|
|=-               Projects: http://freshmeat.net/~danielpb/               -=|
|=-  GnuPG: 7D3B9505   F3C9 553F A1DA 4AC2 5648 23C1 B3DF F742 7D3B 9505  -=| 


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]