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

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

 



On Thu, 2022-12-15 at 09:20 -0600, Benjamin Marzinski wrote:
> On Thu, Dec 15, 2022 at 11:34:20AM +0000, Martin Wilck wrote:
> > 
> > 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.

I am unsure about it, either. A quick experiment shows that the
compiler does not merge multiple const char arrays with the same value
into one memory location, while it does for string constants. (I
suppose this works only for constants defined in the same source
file?).

Also, if you use a pointer (rather than an array), the compiler is
smart enough not to allocate memory for the pointer variable. At least
in simple situations, it's probably the best idea to just use the
string constant without declaring a variable. So you could have written

#define SPACES " \f\r\t\v"
...
		p += strspn(p, SPACES);

This would benefit from gcc's constant merging. In the case at hand,
the compiler output would be nearly the same between all 3 approaches.
But this would change if you started using SPACES in some other
function.

What definitely should not be used is 

        const char spaces[] = " \f\r\t\v";

(without "static") because it causes the array to be allocated on the
stack and re-initialized at every function call.

The moral: we shouldn't try to be smarter than the compiler
(this refers to my review :-/)

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