Re: [PATCH 2/2] tools/xl: rework p9 config parsing

[Juergen Gross <jgross@xxxxxxxx>]

On 20.03.23 18:12, Jason Andryuk wrote:
> On Fri, Mar 17, 2023 at 7:16 AM Juergen Gross <jgross@xxxxxxxx> wrote:
> > Rework the config parsing of a p9 device to use the
> > split_string_into_pair() function instead of open coding it.
> >
> > Signed-off-by: Juergen Gross <jgross@xxxxxxxx>
> > ---
> >   tools/xl/xl_parse.c | 72 ++++++++++++++++++++++-----------------------
> >   1 file changed, 35 insertions(+), 37 deletions(-)
> >
> > diff --git a/tools/xl/xl_parse.c b/tools/xl/xl_parse.c
> > index 2f9dfea05c..715e14f95f 100644
> > --- a/tools/xl/xl_parse.c
> > +++ b/tools/xl/xl_parse.c
> > @@ -2111,54 +2111,52 @@ void parse_config_data(const char *config_source,
> >
> >       if (!xlu_cfg_get_list(config, "p9", &p9devs, 0, 0)) {
> >           libxl_device_p9 *p9;
> > -        char *security_model = NULL;
> > -        char *path = NULL;
> > -        char *tag = NULL;
> > -        char *backend = NULL;
> > -        char *p, *p2, *buf2;
> >
> >           d_config->num_p9s = 0;
> >           d_config->p9s = NULL;
> >           while ((buf = xlu_cfg_get_listitem (p9devs, d_config->num_p9s)) != 
> > NULL) {
> > +            libxl_string_list pairs;
> > +            int len;
> > +
> >               p9 = ARRAY_EXTEND_INIT(d_config->p9s,
> >                                      d_config->num_p9s,
> >                                      libxl_device_p9_init);
> >               libxl_device_p9_init(p9);
> >
> > -            buf2 = strdup(buf);
> > -            p = strtok(buf2, ",");
> > -            if(p) {
> > -               do {
> > -                  while(*p == ' ')
> > -                     ++p;
> > -                  if ((p2 = strchr(p, '=')) == NULL)
> > -                     break;
> > -                  *p2 = '\0';
> > -                  if (!strcmp(p, "security_model")) {
> > -                     security_model = strdup(p2 + 1);
> > -                  } else if(!strcmp(p, "path")) {
> > -                     path = strdup(p2 + 1);
> > -                  } else if(!strcmp(p, "tag")) {
> > -                     tag = strdup(p2 + 1);
> > -                  } else if(!strcmp(p, "backend")) {
> > -                     backend = strdup(p2 + 1);
> > -                  } else {
> > -                     fprintf(stderr, "Unknown string `%s' in 9pfs spec\n", p);
> > -                     exit(1);
> > -                  }
> > -               } while ((p = strtok(NULL, ",")) != NULL);
> > -            }
> > -            if (!path || !security_model || !tag) {
> > -               fprintf(stderr, "9pfs spec missing required field!\n");
> > -               exit(1);
> > +            split_string_into_string_list(buf, ",", &pairs);
> > +            len = libxl_string_list_length(&pairs);
> > +            for (i = 0; i < len; i++) {
> > +                char *key, *value;
> > +                int rc;
> > +
> > +                rc = split_string_into_pair(pairs[i], "=", &key, &value,
> > +                                            isspace);
> > +                if (rc != 0) {
> > +                    fprintf(stderr, "failed to parse 9pfs configuration: %s",
> > +                            pairs[i]);
> > +                    exit(1);
> > +                }
> > +
> > +                if (!strcmp(key, "security_model")) {
> > +                    replace_string(&p9->security_model, value);
> > +                } else if (!strcmp(key, "path")) {
> > +                    replace_string(&p9->path, value);
> > +                } else if (!strcmp(key, "tag")) {
> > +                    replace_string(&p9->tag, value);
> > +                } else if (!strcmp(key, "backend")) {
> > +                    replace_string(&p9->backend_domname, value);
> > +                } else {
> > +                    fprintf(stderr, "Unknown 9pfs parameter '%s'\n", key);
> > +                    exit(1);
> > +                }
> > +                free(key);
> > +                free(value);
> >               }
> > -            free(buf2);
> I think you need libxl_string_list_dispose(&pairs); somewhere around here?
Ah yes, thanks for noticing.


Juergen

Attachment: OpenPGP_0xB0DE9DD628BF132F.asc
Description: OpenPGP public key

Attachment: OpenPGP_signature
Description: OpenPGP digital signature