[FFmpeg-devel] [PATCH] "assert(a && b)" --> "assert(a); assert(b)" for more precise diagnostics, except for libformat

[Hendrik Leppkes]

On Tue, May 14, 2019 at 11:25 PM Adam Richter <adamrichter4 at gmail.com> wrote:
>
> Consider, for example, if you agree that columnization makes this range check
> more recognizable in a glance and makes it easier to spot what the bounds are
> (the sixteen space indentation is taken from the code in which it appeared):
>
>                 av_assert0(par->bits_per_coded_sample >= 0 &&
> par->bits_per_coded_sample <= 8);
>
>                             ...vs...
>
>                 av_assert0(par->bits_per_coded_sample >= 0);
>                 av_assert0(par->bits_per_coded_sample <= 8);
>
> A possible counter-argument to this might be that, in a long sequence
> of assertions, can be informative to group related assertions
> together, which I think is true, but it is possible to get this other
> means, such as by using blank lines to separate express such grouping.
>
> So, Mark, if you decide you are OK with my complete patches, I encourage
> you to let me know.  Otherwise, if there are any of those changes which you
> are OK with, I would like to just to to get those merged.  Finally, if you would
> rather see none of those changes merged (one one to split the assertions in
> libavformat and one was for everywhere else in ffmpeg), please let me know
> about that too, in which case, if no one advocates for their
> inclusion, I'll drop
> my proposal to merge these changes.
>

Unfortunately I have to agree with Mark. asserst that check the same
value or extremely closely related values should stay combined.

>
> Also after this, I may take a look at adding a branch hint to the av_assertN
> macros if nobody objects.
>

Please don't, we don't do branch hints anywhere, and this is a bad
place to start.
If an assert is truely performance sensitive (as in, it makes a
measurable difference), it should probably be moved from assert0 to
assert1, and as such is only enabled in testing builds.

- Hendrik