Opened 3 years ago

Closed 3 years ago

Last modified 3 years ago

#5345 closed defect (fixed)

resample.c has "&& 0" in one of its "if" checks, which disables code

Reported by: dholbert Owned by:
Priority: minor Component: avcodec
Version: git-master Keywords:
Cc: Blocked By:
Blocking: Reproduced by developer: no
Analyzed by developer: no

Description

Summary of the bug:
===================
One of FFmpeg's source files, resample.c, has the following condition:

if (s->input_channels == s->output_channels && s->ratio == 1.0 && 0) {

The "&& 0" at the end there is almost certainly a typo -- it means the condition will always fail, and the code inside of the condition is dead code which will never be visited.

How to reproduce:
=================
(1) Look at this line of source:
https://github.com/FFmpeg/FFmpeg/blob/095347ffe4c73143dbeb7b05cde8891fd1289389/libavcodec/resample.c#L294

(2) Notice the "&& 0" at the end of its "if" condition, which disables it.

RESULT FOR COMPILER OUTPUT:
This bug causes clang 3.8 (maybe older versions as well) to spam this build warning, if you compile with this build warning enabled:

resample.c:296:9 [-Wunreachable-code] code will never be executed

HISTORICAL NOTE:
According to github blame, this change was introduced by michaelni here in 2004:
https://github.com/FFmpeg/FFmpeg/commit/b9d2085ba14aa733503ff02d966204992f46ff00

The first piece of that patch was adding "&& 0" to this condition.

Not sure if that was an accident, or intentionally disabling this condition. If it was intentional, it'd probably be better to explicitly delete this code, or wrap it in "#if 0 ... #endif" so that it's more explicitly-disabled & so the compiler doesn't have to see it.

Change History (6)

comment:1 Changed 3 years ago by dholbert

(BTW: I noticed this bug by seeing this clang build warning go by in my Firefox Nightly compilation output. Firefox's mozilla-central repository has the FFvpx library imported into it.)

comment:2 Changed 3 years ago by cehoyos

Why do you believe there is a typo?

comment:3 Changed 3 years ago by dholbert

Because it disables the condition and all of the code inside of it.

The code has this form right now:

if (condition && condition && 0) {

do stuff;

}

That entire block is dead code, because "&& 0" forces the "if" check to fail, no matter what.

The only explanation for it not being a typo is that the author intended to explicitly disable this check & the code inside it. That may be the case -- but if so, as I noted at the end of the bug report, this code should probably just be deleted (since it's been disabled since 2004) or disabled more explicitly using "#if 0" wrapper.

Last edited 3 years ago by dholbert (previous) (diff)

comment:4 Changed 3 years ago by michael

  • Resolution set to fixed
  • Status changed from new to closed

comment:5 Changed 3 years ago by dholbert

Thanks!

comment:6 Changed 3 years ago by cehoyos

  • Component changed from undetermined to avcodec
  • Priority changed from normal to minor
  • Version changed from unspecified to git-master
Note: See TracTickets for help on using tickets.