Re: [dm-devel] [PATCH 1/3] libmpathutil: simplify set_value

[Benjamin Marzinski <bmarzins@redhat.com>]

On Wed, Dec 14, 2022 at 09:19:45AM +0000, Martin Wilck wrote:
> On Tue, 2022-12-13 at 17:36 -0600, Benjamin Marzinski wrote:
> > alloc_strvec() will never create a strvec with multiple tokens
> > between
> > the quote tokens.  Verify this in validate_config_strvec(), and
> > simplify
> > set_value() by only reading one value after a quote token.
> > 
> > Signed-off-by: Benjamin Marzinski <bmarzins@redhat.com>
> 
> One suggestion below
> 
> 
> > @@ -496,6 +470,10 @@ validate_config_strvec(vector strvec, const char
> > *file)
> >                         if (VECTOR_SIZE(strvec) > i + 1)
> >                                 condlog(0, "ignoring extra data
> > starting with '%s' on line %d of %s", (char *)VECTOR_SLOT(strvec, (i
> > + 1)), line_nr, file);
> >                         return 0;
> > +               } else if (i > 3) {
> > +                       /* There should only ever be one token
> > between quotes */
> > +                       condlog(0, "parsing error starting with '%s'
> > on line %d of %s", str, line_nr, file);
> > +                       return -1;
> >                 }
> >         }
> >         condlog(0, "missing closing quotes on line %d of %s",
> 
> This could be further simplified. We know that strvec[1] is a quote. So
> the only valid possibilities are
> 
>  - strvec[2] is a quote (-> empty string)
>  - strvec[2] is not a quote and strvec[3] is a quote
> 
> The code would be better understandable if we just spell out these
> possibilities rather than using a loop that start at 2 and is left at 3
> already.

Makes sense.

-Ben

> 
> Martin
> 
> 
> 
--
dm-devel mailing list
dm-devel@redhat.com
https://listman.redhat.com/mailman/listinfo/dm-devel