Re: [PATCH V2 0/3] multipath config fixes

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

 



On Thu, Dec 15, 2022 at 11:34:20AM +0000, Martin Wilck wrote:
> On Wed, 2022-12-14 at 15:38 -0600, Benjamin Marzinski wrote:
> > The first two patches are a cleanup and a fix for a memory leak in
> > the
> > config code. The third patch improves multipath's validation of the
> > strings it passes directly into the table, features, path_selector,
> > and
> > hardware_handler.  These three strings all have argument counts, and
> > getting them wrong causes the kernel to parse the table incorrectly.
> > When this happens the table load fails, but the error messages from
> > the
> > kernel are often completely unhelpful.  A bad argument count will
> > cause
> > the rest of the table to be parsed incorrectly, and the kernel might
> > not
> > hit an unworkable token till later in the parsing.  Multipath now
> > makes
> > sure that the count matches the actual number of arguments that it is
> > passing.
> > 
> > V2 Changes (based on suggestions from Martin Wilck):
> > 1/3: unrolled loop in validate_config_strvec() to explicitly check
> > the
> >       possible quote strings
> > 3/3: spaces is now a "const char * const" and doesn't include '\n'
> 
> Hm, my suggestion was wrong. It shouldn't be a pointer at all but an
> array:
> 
> 	static const char spaces[] = " \f\r\t\v";
> 
> "static" makes sure it's only initialized once, and ends
> up in the .rodata section. 

Dumb question. If you explicitly make it an array, does that mean the
compiler will always allocate separate memory for it, even if there is
another identical const array? With multiple pointers to the same const
string, the compiler will often just have one string in memory, which all
the pointers refer to. Not sure if the same thing happens when they're
defined as arrays.

-Ben

> 
> In practice, there's no significant difference between either version. 
> So, for the set:
> 
> Reviewed-by: Martin Wilck <mwilck@xxxxxxxx>
> 
> Martin
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel





[Index of Archives]     [DM Crypt]     [Fedora Desktop]     [ATA RAID]     [Fedora Marketing]     [Fedora Packaging]     [Fedora SELinux]     [Yosemite Discussion]     [KDE Users]     [Fedora Docs]

  Powered by Linux