1124979 - Http2Compression.cpp:1168:45: warning: ‘matchedIndex’ may be used uninitialized in this function [-Wmaybe-uninitialized]

Status: RESOLVED INVALID

(Core :: Networking: HTTP, defect)

Description

[Daniel Holbert [:dholbert]]

Build warning, with clang 3.6 (and probably earlier clang versions as well), in an opt build:
{
netwerk/protocol/http/Http2Compression.cpp:1168:45: warning: ‘matchedIndex’ may be used uninitialized in this function [-Wmaybe-uninitialized]

    DoOutput(kIndex, &inputPair, matchedIndex);
                                              ^
}

Link to source:
http://mxr.mozilla.org/mozilla-central/source/netwerk/protocol/http/Http2Compression.cpp?rev=c97ed39af3f7

From a quick code-skim, it looks to me like we can indeed end up using 'matchedIndex' here without ever initializing it, with the right configuration of args to this function.  Hopefully we just know that we never get such a configuration of args, but it'd be nice if we could make this code more foolproof (or at least add assertions about its expectations, to make it clearer to human readers, if not compilers, that this can never happen).

More details in next comment.

Comment 1

[Daniel Holbert [:dholbert]]

Actually, sorry -- I think I misread the logic, and it's not actually possible for the variable to be used uninitialized.

In particular: if matchedIndex is never set, then the bool 'match' will be false, which means we'll take an early-return.

(This might be clearer -- and maybe more discoverable to the compiler? (not sure) -- if we combined matchedIndex & match into a Maybe<uint32_t>, but that might also hurt readability for those not familiar with Maybe<>)

Anyway, this is probably INVALID because the compiler's just wrong; this is one of the cases it's not sure about, so it warns to be on the safe side.