[FFmpeg-devel] [FFmpeg-cvslog] avcodec/pngdec: Fix () placement

[Reimar Döffinger]

On Wed, Aug 23, 2017 at 10:26:45PM +0200, Alexander Strasser wrote:
> On 2017-08-22 17:23 +0000, Michael Niedermayer wrote:
> > ffmpeg | branch: master | Michael Niedermayer <michael at niedermayer.cc> | Tue Aug 22 18:36:26 2017 +0200| [a2e444d5bb2e3115d3afcc0cca9d1506c90436a2] | committer: Michael Niedermayer
> > 
> > avcodec/pngdec: Fix () placement
> > 
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > 
> > > http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=a2e444d5bb2e3115d3afcc0cca9d1506c90436a2
> > ---
> > 
> >  libavcodec/pngdec.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 
> > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> > index 4fc1c5a062..dce8faf168 100644
> > --- a/libavcodec/pngdec.c
> > +++ b/libavcodec/pngdec.c
> > @@ -858,7 +858,7 @@ static int decode_iccp_chunk(PNGDecContext *s, int length, AVFrame *f)
> >  
> >      length = FFMAX(length - 1, 0);
> >  
> > -    if ((ret = decode_zbuf(&bp, s->gb.buffer, s->gb.buffer + length) < 0))
> > +    if ((ret = decode_zbuf(&bp, s->gb.buffer, s->gb.buffer + length)) < 0)
> 
> IMHO another reason not to do the assignment and the comparison on the same
> line inside the if-condition in C.
> 
> I mean you need the extra parens and if you are not careful enough you will 
> just get it silently wrong like it was above.
> 
> Probably we found enough of such errors to discourage that style in FFmpeg?

It's happening over and over, and I for that reason complain about this
style every time.
But few people so far have been convinced by it that it's a really
bad idea (though IMHO it would still be bad style and readability even
without all the bugs it caused).