search

mackron / dr_libs

Audio decoding libraries for C/C++, each in a single source file.
Other
1.27k stars 206 forks source link

Initialize outputChannels and outputSampleRate to 0 #69

Closed T045T closed 5 years ago

T045T commented 5 years ago

Without this initialization, it's possible to run into situations where a drmp3_config struct is uninitialized, but some function assumes that it is because outputSampleRate is non-zero.

This in turn leads to crashes when the library tries to load an audio file with a sample rate of, for example, ~300MHz and runs into memory problems/segfaults.

bmcdonnell-ionx commented 5 years ago

Did you build that with a C++11 (or later) compiler? I don't believe that is legal C code.

https://stackoverflow.com/questions/225089/why-cant-we-initialize-members-inside-a-structure

T045T commented 5 years ago

I did, and didn't realize default values were a C++11 only thing. I've updated the PR with a commit that initializes the values when the object is created. I went for the more verbose variation to make it clear what gets set to zero, but if you prefer I can make it drmp3_config config = {0, 0};

mackron commented 5 years ago

Hmm, I'm not sure about this one. If you just go down a few lines there's this section:

/* The config can be null in which case we use defaults. */
if (pConfig != NULL) {
    config = *pConfig;
} else {
    DRMP3_ZERO_OBJECT(&config);
}

There's nothing above that using the config object. The proposed change in this pull request will just get overwritten by the code above. This proposal doesn't feel right to me.

T045T commented 5 years ago

You are absolutely right. Turns out the bug I was seeing was an error in calling drmp3. I didn't catch that because it was in a library my project is using rather than my own code.

Sorry for wasting your time, and thanks for pointing me towards the actual bug!