Skip to content

Initialise the fetch-into-temporary locals - #30

Open
heitbaum wants to merge 1 commit into
cdcseacave:masterfrom
heitbaum:fix-maybe-uninitialized-warnings
Open

Initialise the fetch-into-temporary locals#30
heitbaum wants to merge 1 commit into
cdcseacave:masterfrom
heitbaum:fix-maybe-uninitialized-warnings

Conversation

@heitbaum

@heitbaum heitbaum commented Sep 5, 2026

Copy link
Copy Markdown

gcc 16 warns -Wmaybe-uninitialized on all six fetch-into-temporary sites in TinyEXIF.cpp:

TinyEXIF.cpp:895:51: warning: 'altitudeRef' may be used uninitialized [-Wmaybe-uninitialized]
TinyEXIF.cpp:765:62: warning: '_FocalLengthIn35mm' may be used uninitialized [-Wmaybe-uninitialized]
TinyEXIF.cpp:727:47: warning: '_ImageHeight' may be used uninitialized [-Wmaybe-uninitialized]
TinyEXIF.cpp:718:46: warning: '_ImageWidth' may be used uninitialized [-Wmaybe-uninitialized]
TinyEXIF.cpp:560:53: warning: '_RelatedImageWidth' may be used uninitialized [-Wmaybe-uninitialized]
TinyEXIF.cpp:569:54: warning: '_RelatedImageHeight' may be used uninitialized [-Wmaybe-uninitialized]

They are false positives. Fetch() writes its out-parameter only on the return true path, and SetFieldIf() returns fetched unchanged, so a temporary is only read when it was written:

bool Fetch(uint16_t& val) const {
    if (!IsShort() || length == 0)
        return false;
    val = parse16(buf + offs + 8, alignIntel);
    return true;
}

bool EXIFInfo::SetFieldIf(FieldID id, bool fetched) {
    if (fetched)
        SetField(id);
    return fetched;
}

gcc just doesn't carry that correlation across the out-of-line SetFieldIf() call. Initialising the temporaries costs nothing and silences the diagnostic.

altitudeRef needs one extra step: it is declared directly under case 5: with no enclosing block, so adding an initialiser there would make the jumps to case 6: and the later labels cross an initialisation (the declaration is legal today only because it has vacuous initialisation). It is wrapped in a block covering just the declaration and its use, leaving the case 5: label and its break; exactly as they are.

Found building the vendored copy of TinyEXIF in xbmc/imagedecoder.heif (synced to 8c22aff) with gcc 16.2.0 for LibreELEC.

Fetch() writes its out-parameter only when it returns true, and
SetFieldIf() passes that result straight through, so the temporaries are
never actually read uninitialised. gcc does not carry that correlation
across the out-of-line SetFieldIf() call though, and gcc 16 warns on all
six sites:

  TinyEXIF.cpp:895:51: warning: 'altitudeRef' may be used uninitialized
  TinyEXIF.cpp:765:62: warning: '_FocalLengthIn35mm' may be used uninitialized
  TinyEXIF.cpp:727:47: warning: '_ImageHeight' may be used uninitialized
  TinyEXIF.cpp:718:46: warning: '_ImageWidth' may be used uninitialized
  TinyEXIF.cpp:560:53: warning: '_RelatedImageWidth' may be used uninitialized
  TinyEXIF.cpp:569:54: warning: '_RelatedImageHeight' may be used uninitialized

Give the temporaries an initial value. altitudeRef is declared directly
under "case 5:", where an initialiser would make the jumps to the later
case labels cross an initialisation, so scope it in a block covering just
the declaration and its use.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant