[FFmpeg-devel] [PATCH 2/4] cbs: Remove useless initializations

[Reimar Döffinger]

On 03.06.2019, at 14:07, Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:

> Reimar Döffinger:
>> On 03.06.2019, at 00:37, Andreas Rheinhardt <andreas.rheinhardt at gmail.com> wrote:
>> 
>>> Up until now, a temporary variable was used and initialized every time a
>>> value was read in CBS; if reading turned out to be successfull, this
>>> value was overwritten (without having ever been looked at) with the
>>> value read if reading was successfull; on failure the variable wasn't
>>> touched either. Therefore these initializations can be and have been
>>> removed.
>>> 
>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt at gmail.com>
>>> ---
>>> And? What did the ancient compilers say?
>> 
>> Not sure how to read that, but some compilers probably produce "may be used uninitialized" warnings after this change.
>> IMHO it would be better to need evidence of an advantage to remove variable initialization for non-trivial code, even if they are unnecessary. Your commit message doesn't seem to mention one at least.
>> But I am happy to let some maintainer have the last word.
> This comment refers to what Mark said in his review [1] of an earlier
> version of this patchset. He explicitly mentioned that (if his memory
> is right) an older compiler warned without the initializations.
> And I actually thought that there is no need for a further reason to
> remove unnecessary initializations despite the usual ones: Speed and
> code size. E.g. the size of my cbs_mpeg2.o went down from 30421 B to
> 29445 B after this patch; for  cbs_h2645.o the numbers are 249605 B
> and 242245 B. These macros are used a lot (mostly indirectly via other
> macros); see the various cbs_*_syntax_template.c files for that.

Well, your commit message did not mention that.
I kind of assumed that given the compiler does not emit a "may be used uninitialized" warning it figured out that the initialization was unnecessary and would produce identical code.
Maybe the compiler just is more conservative about that warning - or the code size increase might be a bug worth reporting. But I admit finding out which might be a bit too much work.
Either way I would appreciate mentioning explicitly in the commit message code size and speed advantages if you have numbers already anyway.

Thanks!
Reimar