of crashes on systems that check for that.
Message-ID:
<CAKCUQMYdzdtYZRCkkLpivkv4gbZ=WsawwF_4wqj_XgWL87WshA at mail.gmail.com>
Content-Type: text/plain; charset=“iso-8859-1”
Sam applying the patch pretty much answers your points, but I figured
that I should point out some relevant details (especially since I
half-recall reading that you were French, and thus might need some
language tips).
With the little help I tracked the crash down and wrote a test case
that reproduces it perfectly:
#include <mikmod.h>
main()
{
MikMod_RegisterAllDrivers();
MikMod_Init("");
char* d = MikMod_InfoDriver();
if (d) {
printf("%s\n", d);
free(d);
}
MikMod_Exit();
}
The crash happens at free(), where glibc will detect “double-free or
corruption”. This is happening because mikmod uses MikMod_malloc() to
allocate memory for return value, and MikMod_malloc() does some funny
things with allocations and alignments. Replacing free() with
MikMod_free() is enough to stop crashes.
If something sdl uses is broken and then something else uses sdl that would
change things or if your stuff now works but now most other people’s stuff
is broke because of the way it was fixed that would be bad.
You see that bit where mikmod has it’s own free() function, but SDL
wasn’t using it in the necessary situations? That needed to be fixed.
If programs or libraries were depending on that, (which is highly
unlikely, since it was producing crashes), then they need to be fixed.
The change to SDL (regardless of whether it works perfectly with the
first patch) is the correct thing to do, because SDL had the wrong
behavior. If the change breaks something, then it was the fault of the
thing that broke.
Preserving bugs because something uses them is WRONG. I realize that
Microsoft does it with Windows, but (especially with the early
versions, where such anecdotes seem the most common) Windows was
historically considered buggy and prone to viruses, partially because
of those intentionally preserved bugs. Better to deal with it up-front
(especially since API-breakage should still be expected in the 2.0
series) than to let it fester.
What if it’s an
old version and it was fixed two versions ago?
Then the patch gets ignored, unless it is an improvement on the old
fix, in which case Sam & co. decide whether to replace the old patch
with the new. But yes, Paul should have listed the actual version that
he applied the patch to.
One last thing. Your response was toned too harshly. In particular, it
was written as if you were trying to discourage Paul from posting
patches, and if I ignored that implication then it seemed like you
were panicking about the possibilities that you mentioned. I doubt
that you intended the first of those interpretations, but in case the
second was true, I’ll say this:
Stop worrying so much, the cost of those mistakes is very small so
there’s no problem if they happen. Even more importantly, nothing will
improve if nothing changes, and successfully crowd-sourced projects
move faster than ones with only two or three people working on them.
In the words of Douglas Adams: Don’t Panic.> Date: Sun, 22 Jul 2012 17:14:21 -0500
From: R Manard
To: SDL Development List
Subject: Re: [SDL] SDL_mixer: fix mikmod malloc/free mismatch - cause
On Sun, Jul 22, 2012 at 8:14 AM, Paul P Komkoff Jr wrote: