[dm-devel] [PATCH V2 0/3] multipath config fixes

[Benjamin Marzinski]

On Thu, Dec 15, 2022 at 09:20:29AM -0600, Benjamin Marzinski wrote:
> 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

FWIW, I just checked with gcc version 11.3.1. Using:

static const char * const ptr1 = "test";
static const char * const ptr2 = "test";
static const char arr1[] = "test";
static const char arr2[] = "test";

printf("pointers are %s\n", (ptr1 == ptr2) ?  "equal" : "not equal");
printf("arrays are %s\n", (arr1 == arr2) ?  "equal" : "not equal");

I get:

pointers are equal
arrays are not equal

So depending on whether or not we use this multiple times, we can save
ourselves.. a whopping 6 bytes.  At any rate, now I know.

-Ben

> 
> > 
> > In practice, there's no significant difference between either version. 
> > So, for the set:
> > 
> > Reviewed-by: Martin Wilck <mwilck at suse.com>
> > 
> > Martin