On 2014-10-10 10:33 +0200, wm4 wrote: > On Fri, 10 Oct 2014 09:57:20 +0200 > Alexander Strasser <eclip...@gmx.net> wrote: > > > On 2014-10-09 08:13 +0200, Reimar Döffinger wrote: > > > On 7 October 2014 08:41:09 CEST, Peter Ross <pr...@xvid.org> wrote: > > [...] > > > >> > + if ((ret = init_get_bits8(&gb, avpkt->data, avpkt->size)) < 0) > > > >> > + if ((ret = read_map(&gb, &fsets, map_ch_to_felem, > > > >avctx->channels)) < 0) > > > >> > + if ((ret = read_map(&gb, &probs, map_ch_to_pelem, > > > >avctx->channels)) < 0) > > > >> > > > >> I still think putting assignments into an if is really bad idea all > > > >> around. > > > > > > > >No exception for simple error return codes? > > > > > > As Micheal said it's used a lot so no reason for rejecting a patch. > > > However we had loads of bugs due to it, it is hard to read especially for > > > people not that used to C and it only saves a single line. > > > To me personally that's still a completely unreasonable risk/benefit > > > ratio. > > > Normally when we see a feature that is a repeated cause of bugs we avoid > > > it. For some reason people are too attached to this one specifically. > > > I still argue against it because I still believe it makes no sense to use > > > it. > > > Just my opinion though. > > > > +1 > > > > I agree with Reimar. > > > > Harder to read, easier to get wrong. Even worse: if you get it wrong > > it may even go unnoticed for a long time supported by the fact that > > it is harder to read and thus people reading the code will usually not > > notice any errors in such lines. > > > > To be sure: What I wrote above are general assertions, I am not > > commenting on this patch set. > > > > Strange, I thought it was the preferred idiom in ffmpeg. (I don't like > it either, but submitted some patches using this.)
Right, that idiom is used a lot in FFmpeg. That alone does not mean it is preferred. Maybe preferred by some developers and contributors. IMHO we do not need to be that strict and official about it, though at least I would prefer to see the expanded version more often in the future. Alexander
pgpG6HeETbJqX.pgp
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
