Re: [Xen-devel] [PATCH v3 3/3] xl: add support for channels

[Ian Campbell <Ian.Campbell@xxxxxxxxxx>]

On Mon, 2014-06-23 at 15:57 +0100, Ian Jackson wrote:
> David Scott writes ("[PATCH v3 3/3] xl: add support for channels"):
> > This adds support for channel declarations of the form:
> >   channel = [ "name=...,kind=...[,path=...][,backend=...]" ]
> 
> > +    if (!xlu_cfg_get_list (config, "channel", &channels, 0, 0)) {
> > +        d_config->num_channels = 0;
> > +        d_config->channels = NULL;
> > +        while ((buf = xlu_cfg_get_listitem (channels,
> > +                d_config->num_channels)) != NULL) {
> > +            libxl_device_channel *chn;
> > +            char *buf2 = strdup(buf);
> > +            char *p, *p2;
> > +            chn = ARRAY_EXTEND_INIT(d_config->channels, 
> > d_config->num_channels,
> > +                                    libxl_device_channel_init);
> 
> I appreciate that you're just following the example of the vif
> configuration here, but I think this is rather too much open-coded
> string handling.

FWIW I think there are a few places in xl where a precanned parser for
strings of comma-separated key=value syntax would be quite valuable (the
existing one is very disk specific due to the quirks for that syntax).

I don't expect Dave to write one though...

> 
> > +                if (!strcmp(p, "backend")) {
> > +                    free(chn->backend_domname);
> > +                    chn->backend_domname = strdup(p2 + 1);
> 
> At the very least, can we provide a macro or function or something to
> do this ?

I think the existing replace_string already does this.

Ian


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