Bug#948760: berusky2: Compile without warnings
Hello Asher, hello Markus, sorry, I wasn't aware of that patch being applied at Salsa as there was no more activity in 944431, so I thought it went through unnoticed. Sorry for the noise. Kind regards, Bernhard
Bug#948760: berusky2: Compile without warnings
Hello Markus and Bernhard, Markus Koschany writes: > Am 13.01.20 um 22:26 schrieb Bernhard Übelacker: >> Hello Asher, >> maybe you want to incorporate the changes given here: >>https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=944431#31 >> Unfortunately I was too late there. >> >> Then the call to e.g. SetThreadPriority would not needed to get >> commented out, and the "-O0" fix in #944431 could be removed >> again, I guess. > > I had incorporated and applied your patch and removed the -O0 fix again. > I haven't checked though how all this is related to Asher's patch. As Markus says, he's already added Bernhard's fixes. I've written my patch on top of those fixes. Markus Koschany writes: > thanks for the report. Have you considered to forward these patches > upstream and help upstream to implement them? I'm asking because a lot > of users won't benefit from bug fixes if they are only fixed in Debian. I've forked the upstream repository here[1] (because I don't want to use GitHub) and I've sent Martin Stransky an email to let him know. In my repository, I've added Markus's patch for GCC 6 and Bernhard's patch for the 'no return statement in function returning non-void' warning as well as my patches. I've added both of these as separate commits where I've credited you in the commit message. But if you like, I can change the Author to your name instead. Hopefully, these will be accepted upstream and we will be able to drop all patches except the Debian-specific ones (data.patch and docs.patch). Asher Footnotes: [1] https://notabug.org/AsDaGo/berusky2 -- If at first you don't succeed, redefine success. GPG fingerprint: 38F3 975C D173 4037 B397 8095 D4C9 C4FC 5460 8E68 signature.asc Description: PGP signature
Bug#948760: berusky2: Compile without warnings
Am 13.01.20 um 22:26 schrieb Bernhard Übelacker: > Hello Asher, > maybe you want to incorporate the changes given here: >https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=944431#31 > Unfortunately I was too late there. > > Then the call to e.g. SetThreadPriority would not needed to get > commented out, and the "-O0" fix in #944431 could be removed > again, I guess. I had incorporated and applied your patch and removed the -O0 fix again. I haven't checked though how all this is related to Asher's patch. Regards, Markus
Bug#948760: berusky2: Compile without warnings
Hello Asher, maybe you want to incorporate the changes given here: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=944431#31 Unfortunately I was too late there. Then the call to e.g. SetThreadPriority would not needed to get commented out, and the "-O0" fix in #944431 could be removed again, I guess. Kind regards, Bernhard
Bug#948760: berusky2: Compile without warnings
Am 13.01.20 um 03:56 schrieb Asher Gordon: > Package: berusky2 > Version: 0.10+git20170630-3 > Severity: normal > Tags: patch > > Dear Maintainer, > > Currently when Berusky2 is compiled it generates a *lot* of compile > warnings, some of which seem serious. I've fixed all these warnings and > have attached a patch (I've attached it as an attachment rather than > inline because it is quite large (3651 lines) and I don't want to > clutter up the bug report log). > > I've set the severity to "normal" since there are so many warnings and I > suspect that my patch may fix some crashes. But feel free to downgrade > as you see fit. Hello, thanks for the report. Have you considered to forward these patches upstream and help upstream to implement them? I'm asking because a lot of users won't benefit from bug fixes if they are only fixed in Debian. > Since Berusky2 now compiles without warnings, I recommend adding -Werror > to the C{,XX}FLAGS. I guess this would be done by adding it to > DEB_CXXFLAGS_MAINT_APPEND (like you did in 89e7190) and > DEB_CFLAGS_MAINT_APPEND. I think -Werror is never a good idea for Debian packages. Even minor warnings would cause a build failure. Ideally -Werror is used in development to detect and fix warnings but not in production. Again this is something to consider for upstream. > I tested running Berusky2 periodically while writing the patch to make > sure that I didn't introduce any bugs. I also tested it after I finished > writing the patch. But I did not test very thoroughly (I just started it > and made a few moves in a level). However, the only part that seems > likely to introduce new bugs is the replacement of the deprecated ALUT > functions, and sound still works fine. So I'm pretty sure I didn't > introduce any new bugs. > > I have attached the patch after the message. I would feel more comfortable if upstream included and reviewed the patch. It may take a while to find the time to review it myself. Regards, Markus signature.asc Description: OpenPGP digital signature
Bug#948760: berusky2: Compile without warnings
Package: berusky2 Version: 0.10+git20170630-3 Severity: normal Tags: patch Dear Maintainer, Currently when Berusky2 is compiled it generates a *lot* of compile warnings, some of which seem serious. I've fixed all these warnings and have attached a patch (I've attached it as an attachment rather than inline because it is quite large (3651 lines) and I don't want to clutter up the bug report log). I've set the severity to "normal" since there are so many warnings and I suspect that my patch may fix some crashes. But feel free to downgrade as you see fit. Here is a short summary (not meant to be comprehensive) of the changes I have made: * Some functions (such as chdir() and getcwd()) warn if their return values are not checked. I have added checks to all such functions which did not already have checks. * I've added a few assertions where needed to avoid warnings. * I've changed many calls to sprintf() which may overflow to calls to snprintf(). I also check the return value of the snprintf() call since the output may be truncated. * I've replaced the deprecated ALUT functions alutLoadWAVFile() and alutUnloadWAV() with local implementations (s/alut/adas/). * Apparently G++ doesn't like calls to memset() on a non-trivial type (all of these were structs). So I've casted the pointer to (void *) before passing it to memset(). Is there a better way to do this? I don't know C++ very well (the warning said to "use assignment or value-initialization instead"). * Numerous other miscellaneous fixes. The coding style of Berusky2 is inconsistent, so I just tried to use the local style in whatever file (or part of file!) I was editing. I finally (sort of) figured out how to use quilt properly, and I've added a DEP-3 compliant header to my patch. Please remember to update the Bug-Debian (this bug), Reviewed-by (you), and Last-Update headers before you add the patch. Since Berusky2 now compiles without warnings, I recommend adding -Werror to the C{,XX}FLAGS. I guess this would be done by adding it to DEB_CXXFLAGS_MAINT_APPEND (like you did in 89e7190) and DEB_CFLAGS_MAINT_APPEND. I tested running Berusky2 periodically while writing the patch to make sure that I didn't introduce any bugs. I also tested it after I finished writing the patch. But I did not test very thoroughly (I just started it and made a few moves in a level). However, the only part that seems likely to introduce new bugs is the replacement of the deprecated ALUT functions, and sound still works fine. So I'm pretty sure I didn't introduce any new bugs. I have attached the patch after the message. Thanks, Asher -- ...very few phenomena can pull someone out of Deep Hack Mode, with two noted exceptions: being struck by lightning, or worse, your *computer* being struck by lightning. -- Matt Welsh GPG fingerprint: 38F3 975C D173 4037 B397 8095 D4C9 C4FC 5460 8E68 -- System Information: Debian Release: bullseye/sid APT prefers testing-debug APT policy: (500, 'testing-debug'), (500, 'testing') Architecture: amd64 (x86_64) Kernel: Linux 5.4.0-2-amd64 (SMP w/2 CPU cores) Kernel taint flags: TAINT_FIRMWARE_WORKAROUND Locale: LANG=en_US.UTF-8, LC_CTYPE=en_US.UTF-8 (charmap=UTF-8), LANGUAGE=en_US.UTF-8 (charmap=UTF-8) Shell: /bin/sh linked to /bin/dash Init: systemd (via /run/systemd/system) LSM: AppArmor: enabled Versions of packages berusky2 depends on: ii berusky2-data 0.9-2 ii libalut01.1.0-6 ii libc6 2.29-7 ii libgcc1 1:9.2.1-22 ii libgl1 1.1.0-1+b1 ii libglu1-mesa [libglu1] 9.0.1-1 ii libopenal1 1:1.19.1-1+b1 ii libsdl-image1.2 1.2.12-12 ii libsdl1.2debian 1.2.15+dfsg2-5 ii libstdc++6 9.2.1-22 ii libvorbisfile3 1.3.6-2 ii libx11-62:1.6.8-1 ii zlib1g 1:1.2.11.dfsg-1+b1 berusky2 recommends no packages. berusky2 suggests no packages. -- no debconf information Description: Make Berusky2 compile cleanly with no warnings Berusky2 used to generate a *lot* of warnings when compiled. Some of these were not too serious, but some seemed like they could possibly allow buffer overflows. This patch likely fixes many potential crashes as well. Author: Asher Gordon Bug-Debian: TODO Reviewed-by: TODO Last-Update: 2020-01-12 --- This patch header follows DEP-3: http://dep.debian.net/deps/dep3/ Index: berusky2/src/adas/adas.c === --- berusky2.orig/src/adas/adas.c +++ berusky2/src/adas/adas.c @@ -214,7 +214,7 @@ void adas_Init(ADAS_INIT_DATA * p_adas_d memcpy(_data, p_adas_data, sizeof(ADAS_INIT_DATA)); - p_Device = alcOpenDevice((ALCubyte *) p_adas_data->Implementation); + p_Device = alcOpenDevice(p_adas_data->Implementation); if (p_Device) { bDevice = 1; p_Context = alcCreateContext(p_Device, NULL); @@ -222,14 +222,14 @@ void