Skip to content

Commit

Permalink
Fix potential infinite loop in S_PaintChannels
Browse files Browse the repository at this point in the history 
when playing sounds with loop start >= end
(e.g. misc/forcefield.wav from Madfox's kaptlog.zip)
  • Loading branch information
@andrei-drexler
andrei-drexler committed Jun 15, 2024
1 parent b444798 commit b20c27b
Showing 1 changed file with 6 additions and 0 deletions.
6 changes: 6 additions & 0 deletions Quake/snd_mem.c
View file
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,12 @@ sfxcache_t *S_LoadSound (sfx_t *s)
if (!sc)
return NULL;

if (info.loopstart >= info.samples)
{
Con_Warning ("%s has loop start >= end\n", s->name);
info.loopstart = -1;
}

sc->length = info.samples;
sc->loopstart = info.loopstart;
sc->speed = info.rate;
Expand Down

6 comments on commit b20c27b

@andrei-drexler
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@sezero @j0zzz This affects QS and JoeQuake as well. You can repro it by unzipping this file into your id1 dir and running play forcefield in the console, the game will become unresponsive.

@sezero
Copy link

@sezero sezero commented on b20c27b Jun 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but should this not be done in GetWavinfo? Because, if we hit this, info.samples will most likely stay miscalculated in GetWavInfo().

@sezero
Copy link

@sezero sezero commented on b20c27b Jun 16, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I.e. I suggest the following on top of this patch: what do you think?

diff --git a/Quake/snd_mem.c b/Quake/snd_mem.c
index a7962e7..5bb8ed8 100644
--- a/Quake/snd_mem.c
+++ b/Quake/snd_mem.c
@@ -149,12 +149,6 @@ sfxcache_t *S_LoadSound (sfx_t *s)
 	if (!sc)
 		return NULL;
 
-	if (info.loopstart >= info.samples)
-	{
-		Con_Warning ("%s has loop start >= end\n", s->name);
-		info.loopstart = -1;
-	}
-
 	sc->length = info.samples;
 	sc->loopstart = info.loopstart;
 	sc->speed = info.rate;
@@ -343,6 +337,13 @@ wavinfo_t GetWavinfo (const char *name, byte *wav, int wavlength)
 	data_p += 4;
 	samples = GetLittleLong() / info.width;
 
+	if (info.loopstart >= info.samples)
+	{
+		info.loopstart = -1;
+		info.samples = 0;
+		Con_Warning ("%s has loop start >= end\n", name);
+	}
+
 	if (info.samples)
 	{
 		if (samples < info.samples)

@andrei-drexler
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's a good point. I'd move the fix a bit lower, though, after the if (info.samples) block, since info.samples gets overwritten in the else branch.

@andrei-drexler
Copy link
Owner Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Force-pushed 3f41b0d, with the new location.

@j0zzz
Copy link

@j0zzz j0zzz commented on b20c27b Jul 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Merged the fix to joequake, thx for the heads up!

Please sign in to comment.