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

Re: [Xen-devel] [PATCH V2] libxl: support custom block hotplug scripts



On Thu, 2012-08-02 at 17:20 +0100, Ian Jackson wrote:
> Ian Campbell writes ("[PATCH V2] libxl: support custom block hotplug 
> scripts"):
> > libxl: support custom block hotplug scripts
> 
> Wow.  Thanks.  Everything looks good apart from this:
> 
> >                      DPC->had_depr_prefix=1; DEPRECATE("use `script=...'");
> > -                   SAVESTRING("script", script, yytext);
> > -               }
> > +                    if (DPC->disk->script) {
> > +                        if (*DPC->disk->script) {
> > +                            xlu__disk_err(DPC,yytext,"script respecified");
> > +                            return 0;
> > +                        }
> > +                        /* do not complain about overwriting empty strings 
> > */
> > +                        free(DPC->disk->script);
> > +                    }
> > +                    DPC->disk->script = malloc(strlen("block-")
> > +                                               +strlen(yytext) + 1);
> > +                    strcpy(DPC->disk->script, "block-");
> > +                    strcat(DPC->disk->script, yytext);
> 
> Isn't this very like the contents of the savestring() function ?

> Ie you could do:
>         char *newscript;
>         asprintf(&newscript, ...);
>         savestring(DPC, "script respecified", &DPC->disk->script, newscript);
>         free(newscript);
> 
> Other places in xl use asprintf so you can use it here.

I hadn't realised asprintf was fair game. I'll rework along these lines.

> > +                }
> 
> Is this one-character indentation change intentional ?

There's a few stray hard tabs in this file (which is predominantly using
8 space indendation). I think I fixed it in the blocks I was changing
which will look like a 1 char reindent in an MUA. 

Ian.



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxx
http://lists.xen.org/xen-devel


 


Rackspace

Lists.xenproject.org is hosted with RackSpace, monitoring our
servers 24x7x365 and backed by RackSpace's Fanatical Support®.