mackron / dr_libs

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

dr_flac.h: Fixed a "use of uninitializated value" warning #260

Closed Wohlstand closed 10 months ago

Wohlstand commented 10 months ago

Some compilers do report the absence of initialisation here as a possible use of uninitialized value.

mackron commented 10 months ago

Thanks. I've yet to see this specific warning myself. Happy to merge this, but before I do, did you by chance check the code as to why specifically the compiler was reporting this, and where it was determining it was being used without initialization? I was just wanting to make sure there's no subtle underlying bug somewhere in there.

I will take a look at this code when I get a chance to ensure there's nothing suspicious going on there.

Wohlstand commented 10 months ago

It's one of GCC versions causes this warning, I don't remind what the warning... But I should try to cause it again... Something like GCC 13 gives it. If I reproduce, I'll show you it.

Wohlstand commented 10 months ago

Okay, I finally reproduced it, and it happens on the switch(blockType) line:

In file included from /home/vitaly/_git_repos/SDL_Mixer_X/src/codecs/music_drflac.c:47:
/home/vitaly/_git_repos/SDL_Mixer_X/src/codecs/dr_libs/dr_flac.h: In function 'drflac__read_and_decode_metadata':
/home/vitaly/_git_repos/SDL_Mixer_X/src/codecs/dr_libs/dr_flac.h:6506:9: warning: 'blockType' may be used uninitialized [-Wmaybe-uninitialized]
 6506 |         switch (blockType)
      |         ^~~~~~
Wohlstand commented 10 months ago

The compiler version is GCC 13.2.0 where this warning had been reproduced.

mackron commented 10 months ago

Thanks. I've reviewed the code in dr_flac and that's definitely an erroneous report by GCC. I can't see any way where that variable is being used without being initialized. Regardless, it's a good idea to initialize it to 0 anyway so I'm happy to get this merged. I've merged it into the dev branch and will get a release done to the master branch soon.

Wohlstand commented 10 months ago

I guess, that because it didn't detected that variable had been actually initialized in the function below, and it noted that variable is POSSIBLY uninitialized, because it's not a static analyzer that deeply scans the code and builds the whole workflow.

mackron commented 10 months ago

Yeah my suspicion is that it's getting caught up on the early return DRFLAC_FALSE part. In any case, I've also updated the master branch with this fix and bumped the version number. Thanks for the report.