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

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

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