Re: [Qemu-devel] [PULL for-1.7 v2 3/6] acpi-build: fix build on glib < 2.22

[Michael S. Tsirkin]

On Tue, Nov 26, 2013 at 07:18:04AM +1000, Richard Henderson wrote:
> On 11/26/2013 06:54 AM, Michael S. Tsirkin wrote:
> >>>>> +    char s[] = "XXXX";
> >>>>
> >>>> char s[5];
> >>>>
> >>
> >> Then do something like
> >>
> >>   char s[sizeof("XXXX")];
> >>
> >> so that the actual initialization doesn't happen.
> > Why? As an optimization?
> 
> How about failing to pessimize?
> 
> With your initialization you're forcing the compiler to do:
> 
>   char s[5];
>   memcpy(s, "XXXX\0", 5);
> 
> possibly with the memcpy expanded inline.
> Since we pass the address of S to vnsprintf, the compiler has to assume that
> memory is read, and thus the initialization is needed.  It can never be
> optimized away.

Who cares?
It runs once on initialization.
You worry about a cost of memcpy?
FYI we are doing a crazy number of memory
allocations in this code.
I'm much more concerned with the fact that unlike the
glib based variant we had before this is doing
pointer math and relies on correct parameters
to be passed to snprintf to avoid buffer overruns.

I haven't found a way around that that will also
keep old systems happy, and the snippet is tiny so
maybe it's not too bad ...

> 
> > I'm not quite sure this doesn't mean we are using VLA which I'd rather not.
> > Would need to look at language spec ... simple initialization is shorter
> > and more obviously correct.
> 
> I'm quite sure that using sizeof does not imply a VLA.
> 
> 
> 
> r~