Re: [Xen-devel] Build errors with latest xen-unstable from staging

[Andre Przywara <andre.przywara@xxxxxxx>]

Ian Campbell wrote:

> On Sun, 2011-02-06 at 19:01 +0000, Kamala Narasimhan wrote:
> > FYI - Pulled the latest xen-unstable from staging to sync some patches and got
> > these trivial errors while compiling -
> >
> > xl_cmdimpl.c: In function âprint_domain_vcpuinfoâ:
> > xl_cmdimpl.c:3351: warning: âfirstsetâ may be used uninitialized in this 
> > function
> > xl_cmdimpl.c:3351: note: âfirstsetâ was declared here
> > xl_cmdimpl.c:3350: warning: âbitmaskâ may be used uninitialized in this function
> > xl_cmdimpl.c:3350: note: âbitmaskâ was declared here
> > xl_cmdimpl.c:3350: warning: âpmapâ may be used uninitialized in this function
> > xl_cmdimpl.c:3350: note: âpmapâ was declared here
> >
> > GCC version - 4.2.4.  Initializing the three variables it complained about fixed
> > the issue.
> They are actually initialised before use, during the first pass through
> the for loop when i==0 and state==0, but I can see how gcc would be
> unable to figure that out (in fact I'm not sure about firstset myself).
I agree with Ian, looking through the code again I can confirm that they 
are _not_ used uninitialized. Even firstset is set when the state 
switches from 0 to 1 and is only used later in state 1 and 3, so it is safe.
It seems like the compiler is not smart enough to recognize this, 
especially the modulo operation could be a barrier for the analysis, I 
think.
According to the manpage those warning are only emitted on -O, which I 
left out for my testing, so I didn't spot this warning. Sorry.
To appease the compiler I would ack the fix, though from an academic 
point of view it is not necessary.
> In the Linux kernel they have a macro to annotate such instances:
>         /*
>          * A trick to suppress uninitialized variable warning without 
> generating any
>          * code
>          */
>         #define uninitialized_var(x) x = x
>         
> Do we want something similar?
I don't think so. x = x looks more like a hack, if we can easily 
initialize the variables, we should do so.
Regards,
Andre.

> >   If this trivial change should require a signed off line, here it is
> > - Signed-off-by: Kamala Narasimhan <kamala.narasimhan@xxxxxxxxxx>
> Always just assume a change does.
>
> Ian.
> > diff -r 7ada6faef565 tools/libxl/xl_cmdimpl.c
> > --- a/tools/libxl/xl_cmdimpl.c  Sun Feb 06 17:26:31 2011 +0000
> > +++ b/tools/libxl/xl_cmdimpl.c  Sun Feb 06 13:53:50 2011 -0500
> > @@ -3347,8 +3347,8 @@ static void print_bitmap(uint8_t *map, i
> >  static void print_bitmap(uint8_t *map, int maplen, FILE *stream)
> >  {
> >      int i;
> > -    uint8_t pmap, bitmask;
> > -    int firstset, state = 0;
> > +    uint8_t pmap = 0, bitmask = 0;
> > +    int firstset = 0, state = 0;
> >
> >      for (i = 0; i < maplen; i++) {
> >          if (i % 8 == 0) {
> >
> > Kamala
--
Andre Przywara
AMD-Operating System Research Center (OSRC), Dresden, Germany



_______________________________________________
Xen-devel mailing list
Xen-devel@xxxxxxxxxxxxxxxxxxx
http://lists.xensource.com/xen-devel