warnings in debug printf format strings on obscure platforms
Brought to you by:
sobukus
Menu
▾
▴
1 Attachments
#309 warnings in debug printf format strings on obscure platforms
Building libmpg123 for DJGPP (GCC 10.2 for DOS 32bit protected mode) results in:
[CC] include/mpg123/src/libmpg123/frame.c include/mpg123/src/libmpg123/frame.c: In function 'INT123_frame_gapless_update': include/mpg123/src/libmpg123/frame.c:890:18: warning: format '%li' expects argument of type 'long int', but argument 3 has type 'off_t' {aka 'int'} [-Wformat=] 890 | fprintf(stderr, "\nWarning: Real sample count %"OFF_P" differs from given gapless sample count %"OFF_P". Frankenstein stream?\n" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 891 | , total_samples, gapless_samples); | ~~~~~~~~~~~~~ | | | off_t {aka int} In file included from include/mpg123/src/libmpg123/mpg123lib_intern.h:25, from include/mpg123/src/libmpg123/frame.c:10: include/mpg123/src/compat/compat.h:133:18: note: format string is defined here 133 | # define OFF_P "li" include/mpg123/src/libmpg123/frame.c:890:18: warning: format '%li' expects argument of type 'long int', but argument 4 has type 'off_t' {aka 'int'} [-Wformat=] 890 | fprintf(stderr, "\nWarning: Real sample count %"OFF_P" differs from given gapless sample count %"OFF_P". Frankenstein stream?\n" | ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ 891 | , total_samples, gapless_samples); | ~~~~~~~~~~~~~~~ | | | off_t {aka int} In file included from include/mpg123/src/libmpg123/mpg123lib_intern.h:25, from include/mpg123/src/libmpg123/frame.c:10: include/mpg123/src/compat/compat.h:133:18: note: format string is defined here 133 | # define OFF_P "li"
Also, in id3.c:1217: debug2("got my pos: %zu - %zu", realsize, pos); uses %zu and %zu while realsize and pos are both unsigned long, which requires %lu.
We had some compiler on some platform warn on this in the past in libopenmpt, sadly I did not record in the changelog which platform this happened on, so I cannot reproduce right now. The fix is obviously correct anyway.
Suggested fix against svn trunk attached.
Oh, so you catched be right before releaseing 1.27.1. I guess I should wait a bit for what you also find … you got some platform/toolchain left to test?
And … does mpg123 itself still work in the DOS environment? If not, which parts?
Well, we caught these in the past (these are not new issues), but I forgot to report them.
These are all code changes we currently have in our repository, so no further bug reports for now.
We (https://openmpt.org / https://lib.openmpt.org/) are actually only building libmpg123 itself (and that with our own build system), so I have no idea whether the mpg123 utility would still work in DOS ;-). libmpg123 is only a second optional choice for us in DOS anyway because of the license. For platforms with only static linking, minimp3 (https://github.com/lieff/minimp3) is our default mp3 decoder. As far as our (limited) testing goes, libmpg123 works fine in DOS though.
As for other obscure platforms, we also support building for WebAssembly and JavaScript with emscripten.
Ah, yes, minimp3. Had a quick look once and was relieved that it is not really a performance match for libmpg123;-)
Please note that the LGPL doesn't prohibit static linking. All you have to do is to possibly enable the user to re-link the binary with a differing version of the LGPL library. This could be achieved in various ways. Actually, dynamic linking on a platform could be considered a convencience shortcut only.
Sure, we could easily distribute object files that would allow linking a different libmpg123. However, we prefer to keep things simple in this regard, and in particular do not want to "trap" downstream users with a default configuration that links libmpg123 statically and would require them to also distribute object files for their binaries as well (yes, rare and remote situation, but we just prefer to not ship statically linked LGPL code in the default configurations).
After all, our project is BSD-3-Clause licensed, which in general allows downstream users to modify whatever they want and distribute binaries without having to share their modifications.
With minimp3, we have a solution that works great for us. For users who prefer performance, they can always build with ibmpg123 instead and deal with the LGPL requirements themselves.
On platforms that allow dynamic linking, we default to libmpg123. On Windows, we just ship it as a separate DLL for simplicity.
It's in 1.27.2