Bug#948760: berusky2: Compile without warnings

2020-01-14 Thread Bernhard Übelacker
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

2020-01-13 Thread Asher Gordon
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

2020-01-13 Thread Markus Koschany
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

2020-01-13 Thread 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.

Kind regards,
Bernhard



Bug#948760: berusky2: Compile without warnings

2020-01-13 Thread Markus Koschany


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

2020-01-12 Thread 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.

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