[FFmpeg-devel] [PATCH 1/2] libwebpenc_animencoder: zero initialize the WebPAnimEncoderOptions struct

[Michael Niedermayer]

On Thu, Mar 17, 2016 at 05:20:00PM +0100, wm4 wrote:
> On Thu, 17 Mar 2016 13:15:23 -0300
> James Almer <jamrial at gmail.com> wrote:
> 
> > On 3/17/2016 1:02 PM, Michael Niedermayer wrote:
> > > On Thu, Mar 17, 2016 at 11:06:29AM -0300, James Almer wrote:  
> > >> On 3/17/2016 4:32 AM, wm4 wrote:  
> > >>> On Thu, 17 Mar 2016 01:03:49 -0300
> > >>> James Almer <jamrial at gmail.com> wrote:
> > >>>  
> > >>>> This zeroes the WebPAnimEncoderOptions.verbose field, silencing library info messages
> > >>>> printed to stderr.
> > >>>>
> > >>>> Signed-off-by: James Almer <jamrial at gmail.com>
> > >>>> ---
> > >>>>  libavcodec/libwebpenc_animencoder.c | 2 +-
> > >>>>  1 file changed, 1 insertion(+), 1 deletion(-)
> > >>>>
> > >>>> diff --git a/libavcodec/libwebpenc_animencoder.c b/libavcodec/libwebpenc_animencoder.c
> > >>>> index d7437a9..35c456a 100644
> > >>>> --- a/libavcodec/libwebpenc_animencoder.c
> > >>>> +++ b/libavcodec/libwebpenc_animencoder.c
> > >>>> @@ -41,7 +41,7 @@ static av_cold int libwebp_anim_encode_init(AVCodecContext *avctx)
> > >>>>      int ret = ff_libwebp_encode_init_common(avctx);
> > >>>>      if (!ret) {
> > >>>>          LibWebPAnimContext *s = avctx->priv_data;
> > >>>> -        WebPAnimEncoderOptions enc_options;
> > >>>> +        WebPAnimEncoderOptions enc_options = { 0 };
> > >>>>          WebPAnimEncoderOptionsInit(&enc_options);
> > >>>>          // TODO(urvang): Expose some options on command-line perhaps.
> > >>>>          s->enc = WebPAnimEncoderNew(avctx->width, avctx->height, &enc_options);  
> > >>>
> > >>> Does this mean it was reading uninitialized values from the stack?  
> > >>
> > >> Apparently. Should i backport this?  
> > > 
> > > please backport if older releases are affected  
> > 
> > 2.7 and newer, yes.
> > 
> > > 
> > > also my gcc is not fully happy about this:
> > > libavcodec/libwebpenc_animencoder.c: In function ‘libwebp_anim_encode_init’:
> > > libavcodec/libwebpenc_animencoder.c:44:9: warning: missing braces around initializer [-Wmissing-braces]
> > > libavcodec/libwebpenc_animencoder.c:44:9: warning: (near initialization for ‘enc_options.anim_params’) [-Wmissing-braces]
> > >   
> > 
> > Odd, GCC 5.3 doesn't complain, at least not when targeting mingw-w64.
> > anim_params is a WebPMuxAnimParams struct within the WebPAnimEncoderOptions struct,
> > and WebPAnimEncoderOptionsInit() initializes it to default values anyway, so probably
> > not an issue.
> > 
> > In any case, would something like 
> > 
> > WebPAnimEncoderOptions enc_options = { .anim_params = { 0 }, 0 };
> > 
> > be better, or is it too ugly? Alternatively i could just memset the whole thing to 0.
> 
> You don't need that. Your original patch is perfectly valid C.

avoiding code that generates warnings improves the signal to noise
ratio.
Its easier to use warnings to spot bugs when code that is perfectly
valid but generates a warning is avoided.

Is this type of warning ever usefull ? if not maybe it could
be disabled instead as well ...

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

While the State exists there can be no freedom; when there is freedom there
will be no State. -- Vladimir Lenin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20160317/5a0cbfd0/attachment.sig>