[FFmpeg-devel] Curly braces around single statements (was avformat/avidec: Fix memleak when error) happens after creating DV stream

[Mark Thompson]

On 20/08/2020 22:20, Alexander Strasser wrote:
> Please pardon me for bringing this up in the context of this patch.
> No objections or particular opinion regarding this instance of the
> problem.
> 
> Though thinking more globally, I believe it would have a beneficial
> impact to add the curly braces everywhere; even where they would not
> be required because of the one-statement exception.
> 
> It might be a bit longer regarding vertical space consumption, but
> I'm sure the advantages would outweigh this disadvantage over time.
> Also because we don't put the opening brace on a line of its own,
> it would not consume so much more vertical space.
> 
> Advantages I see are:
> 
> 1. enables easier experimentation and debugging

In my opinion this is not a significant benefit, the overhead of any edits while doing this is trivial.  (Others may disagree, but it seems worth noting which parts I agree with.)

> 2. future changes are easier to write and create smaller diffs

I agree that this is a point in favour.

> 3. completely eliminates dangling-else problems

I think this is irrelevant, because compilers have already solved it:

$ cat test-dangling-else.c

int f(int a, int b)
{
     if (a)
         if (b)
             return 1;
       else
           return 2;

     return 0;
}
$ gcc -c -Wall test-dangling-else.c
test-dangling-else.c: In function ‘f’:
test-dangling-else.c:4:8: warning: suggest explicit braces to avoid ambiguous ‘else’ [-Wdangling-else]
     4 |     if (a)
       |        ^

> 
> 
> Just wanted to hear how other developers currently feel about this.

The freeform nature of C is often helpful to make code look nicer and such a requirement would make some useful patterns worse, so I oppose such a change.

To offer an example, consider the commonish code pattern:

if      (a) p = x;
else if (b) q = y;
else if (c) r = z;

with some alignment of related subexpressions.  Making uniform use of braces and newlines mandatory makes this actively worse however you do it.

(Some randomly-chosen similar examples: <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/mpegaudiodsp_template.c;h=e531f8a904b14ad2f5cc6e59ad608bfe64b50065;hb=HEAD#l236>, <http://git.videolan.org/?p=ffmpeg.git;a=blob;f=libavcodec/error_resilience.c;h=ca2287198be69505a03f12dcbe9cff2b485de769;hb=HEAD#l465>.)

- Mark