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@xxxxxxxx>
> >
> > Martin
--
dm-devel mailing list
dm-devel@xxxxxxxxxx
https://listman.redhat.com/mailman/listinfo/dm-devel
