[FFmpeg-devel] [PATCH] [RFC] fix 'may be used uninitialized' warnings

[Michael Niedermayer]

On Mon, Feb 02, 2009 at 10:15:44PM +0100, Diego Biurrun wrote:
> On Mon, Feb 02, 2009 at 04:02:42PM +0100, Michael Niedermayer wrote:
> > On Mon, Feb 02, 2009 at 12:59:19PM +0100, Diego Biurrun wrote:
> > > On Mon, Feb 02, 2009 at 12:49:43PM +0100, Michael Niedermayer wrote:
> > > > On Mon, Feb 02, 2009 at 11:43:16AM +0100, Diego Biurrun wrote:
> > > > > Here is a patch to fix all but one 'may be used initialized' warning
> > > > > in FFmpeg.  Regression tests pass, so this cannot have broken things
> > > > > too badly, but some of it may nonetheless be suspicious.
> > > > > 
> > > > > Please review, I will commit individual hunks as they get approved,
> > > > > not before.
> > > > > 
> > > > > --- libavcodec/motion_est.c	(revision 16938)
> > > > > +++ libavcodec/motion_est.c	(working copy)
> > > > > @@ -536,7 +536,7 @@
> > > > >      int block;
> > > > > -    int P[10][2];
> > > > > +    int P[10][2] = {0, 0};
> > > > >      int dmin_sum=0, mx4_sum=0, my4_sum=0;
> > > > > @@ -686,7 +686,7 @@
> > > > >      int block;
> > > > > -    int P[10][2];
> > > > > +    int P[10][2] = {0, 0};
> > > > >      uint8_t * const mv_penalty= c->current_mv_penalty;
> > > > 
> > > > P[10][2] occurs 6 times, why is gcc complaining just about these 2 ?
> > > 
> > > I wondered myself.  I think array entries that are not initialized get
> > > initialized to 0 automatically.
> > > 
> > > > also it would help me review if you could add -p to the switches so that the
> > > > function names are vissble
> > > 
> > > I'll resend a patch with that switch later.
> > > 
> > 
> > > > > --- libavcodec/eatgv.c	(revision 16938)
> > > > > +++ libavcodec/eatgv.c	(working copy)
> > > > > @@ -63,7 +63,7 @@
> > > > >      unsigned char *dst_end = dst + width*height;
> > > > > -    int size,size1,size2,offset,run;
> > > > > +    int size, size1, size2, offset = 0, run;
> > > > >      unsigned char *dst_start = dst;
> > > > >  
> > > > >      if (src[0] & 0x01)
> > > > 
> > > > very obviously false positive, it can never be read without prior init
> > > 
> > > What about all those falso positives?  Are those hunks OK to commit?
> > 
> > no
> > IMHO if a warning is wrong it should be fixed without changing the generated
> > code.
> 
> How?


#ifdef I_WANT_ALL_WARNINGS //not set by default!
#   define AV_UNINIT(x) x
#elif LANGUAGE_LAWYER //not set by default!
#   define AV_UNINIT(x) x=0
#else
#   define AV_UNINIT(x) x=x
#endif

int AV_UNINIT(x);

either that or add -Wno-whateveritiscalled


> 
> > Of course there may be desireable changes in the code that might
> > remove a warning as a sideeffect but adding =0 is not.
> > If theres a =0 then the reader thinks it IS used which can in principle
> > lead to confusion.
> 
> How about adding comments like
> 
> // silence gcc
> 
> to those cases?  

thats better than not having such comment


> We have similar warning workarounds in a lot of places
> already...

sadly ...

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

He who knows, does not speak. He who speaks, does not know. -- Lao Tsu
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090202/4ac1941a/attachment.pgp>