[FFmpeg-devel] [RFC] Direct Stream Transfer (DST) decoder

[Alexander Strasser]

On 2014-10-09 08:13 +0200, Reimar Döffinger wrote:
> On 7 October 2014 08:41:09 CEST, Peter Ross <pross at 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.


  Alexander
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20141010/504383ba/attachment.asc>