SDL_JoystickInit error

SDL Development
1

I was browsing the source, when I noticed that there’s a problem with
SDL_JoystickInit() in SDL12/src/joystick/SDL_joystick.c

If malloc() fails inside the function, it’ll still return 0; failing to have
inited the SDL_joysticks variable

it’s currently:
int SDL_JoystickInit(void)
{
int arraylen;
int status;
SDL_numjoysticks = 0;
status = SDL_SYS_JoystickInit();
if ( status >= 0 ) {
arraylen = (status+1)*sizeof(*SDL_joysticks);
SDL_joysticks = (SDL_Joystick **)malloc(arraylen);
if ( SDL_joysticks == NULL ) {
SDL_numjoysticks = 0;
} else {
memset(SDL_joysticks, 0, arraylen);
}
SDL_numjoysticks = status;
status = 0;
}
default_joystick = NULL;
return(status);
}

Would this be an ok patch?:

int SDL_JoystickInit(void)
{
int arraylen;
int status;

     SDL_numjoysticks = 0;
     status = SDL_SYS_JoystickInit();
     if ( status >= 0 ) {
             arraylen = (status+1)*sizeof(*SDL_joysticks);
             SDL_joysticks = (SDL_Joystick **)malloc(arraylen);
             if ( SDL_joysticks == NULL ) {
                     status = -1;
             } else {
                     memset(SDL_joysticks, 0, arraylen);
                     SDL_numjoysticks = status;
                     status = 0;
             }
     }
     default_joystick = NULL;
     return(status);

}

2

If malloc() fails inside the function, it’ll still return 0; failing to have
inited the SDL_joysticks variable
…
SDL_joysticks = (SDL_Joystick **)malloc(arraylen);
if ( SDL_joysticks == NULL ) {
SDL_numjoysticks = 0;

Running out of memory at this point is not a fatal condition; we just
say there’re no joysticks available. Otherwise, SDL_Init() would fail,
which would be needlessly fatal in many apps. Those that require joysticks
should be looking at SDL_NumJoysticks() for guidance, and not SDL_Init().

Granted, if malloc() is failing at this point, you probably aren’t getting
much further anyhow, but I don’t think this code makes the wrong choice.

–ryan.

3

But thats my point…
SDL_numjoysticks isn’t set to say that there are no joysticks after failing.
It sets SDL_numjoysticks to the current count of joysticks, and leaves
SDL_joysticks NULL.

This certainly means SDL_JoystickOpen will break - you cant index NULL…
and SDL_NumJoysticks will return the true count, when it should say 0.

But yes, as you say… if the joystick malloc fails, this is the least of your
problem :stuck_out_tongue:

Regards,
John

Ryan C. Gordon wrote:>>If malloc() fails inside the function, it’ll still return 0; failing to have

inited the SDL_joysticks variable
…
SDL_joysticks = (SDL_Joystick **)malloc(arraylen);
if ( SDL_joysticks == NULL ) {
SDL_numjoysticks = 0;

Running out of memory at this point is not a fatal condition; we just
say there’re no joysticks available. Otherwise, SDL_Init() would fail,
which would be needlessly fatal in many apps. Those that require joysticks
should be looking at SDL_NumJoysticks() for guidance, and not SDL_Init().

Granted, if malloc() is failing at this point, you probably aren’t getting
much further anyhow, but I don’t think this code makes the wrong choice.

–ryan.

SDL mailing list
SDL at libsdl.org
http://www.libsdl.org/mailman/listinfo/sdl

4

But thats my point…
SDL_numjoysticks isn’t set to say that there are no joysticks after failing.

Doh, I misread the code. Sorry about that.

I put a fix in CVS. Thanks for pointing this out!

–ryan.

5

I’m with John on this one. Take for example if SDL_SYSJoystickInit()
returns 2, and then malloc fails, which returns NULL.

After if … else block is done, the variables will look like this:

SDL_numjosticks = 0
status = 0
SDL_joysticks = NULL

But then after that block, SDL_numjoysticks will equal status NO MATTER
WHAT, so that’s 2. So at the end of this function, the variables will
stand like this:

SDL_numjoysticks = 2; // From SDL_numjosticks = status;
status = 0; // From status = 0;
SDL_joysticks = NULL; // From FAILED malloc call

John’s reasoning looks clean. And the real killer of this one is that
if malloc does NOT fail, the output from the function looks exactly the
same. So there’s no way to detect this type of failure from your
program. So it’s a logical failure in the if … else branches, and
John looks like he fixed it, since if malloc failed, you could detect it
through the altered status value in his fix.

Now, granted, if malloc fails, your program is probably dead anyways,
but it’s still a bug, even if this isn’t the one that ultimately dooms a
program. John’s fix looks clean to me.

Kevin Frandsen
@Kevin_Frandsen