Strange code in SDL_SendKeyboardText

SDL Development
1

I’ve been tracking down an access violation I get when sending keyboard input to an SDL window, and I tracked it to SDL_SendKeyboardText in SDL_keyboard.c. Specifically, this block:

    event.text.windowID = keyboard->focus ? keyboard->focus->id : 0;
    event.text.which = (Uint8) index;
    SDL_strlcpy(event.text.text, text, SDL_arraysize(event.text.text));
    event.text.windowID = keyboard->focus->id;

What’s going on here? First, we explicitly contemplate the possibility that keyboard->focus might be NULL and account for it. Then we fill in a few other fields, then we repeat the earlier assignment, without the NULL check and things explode because it’s null.

No relevant comments, so I can’t do much to read the original coder’s mind. Is this just something that someone replaced and forgot to take out? I think that last line should be removed.

2

That sounds reasonable :slight_smile:

Jonny DOn Thu, May 13, 2010 at 2:27 PM, Mason Wheeler wrote:

I’ve been tracking down an access violation I get when sending keyboard
input to an SDL window, and I tracked it to SDL_SendKeyboardText in
SDL_keyboard.c. Specifically, this block:

    event.text.windowID = keyboard->focus ? keyboard->focus->id : 0;
    event.text.which = (Uint8) index;
    SDL_strlcpy(event.text.text, text, SDL_arraysize(event.text.text));
    event.text.windowID = keyboard->focus->id;

What’s going on here? First, we explicitly contemplate the possibility
that keyboard->focus might be NULL and account for it. Then we fill in a
few other fields, then we repeat the earlier assignment, without the NULL
check and things explode because it’s null.

No relevant comments, so I can’t do much to read the original coder’s
mind. Is this just something that someone replaced and forgot to take out?
I think that last line should be removed.

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

3

It looks to me like what I like to call “A text editing turd” or just
"Text Turd" for short. A little bit of code that got left in because
someone didn’t do a good job of cleaning up. Sort of like the old joke
about how the starship enterprise is like a piece of toilet paper…

If that is where the error is occurring take it out.

Bob PendletonOn Thu, May 13, 2010 at 1:27 PM, Mason Wheeler wrote:

I’ve been tracking down an access violation I get when sending keyboard
input to an SDL window, and I tracked it to SDL_SendKeyboardText in
SDL_keyboard.c.? Specifically, this block:

??? event.text.windowID = keyboard->focus ? keyboard->focus->id : 0;
??? event.text.which = (Uint8) index;
??? SDL_strlcpy(event.text.text, text, SDL_arraysize(event.text.text));
??? event.text.windowID = keyboard->focus->id;

What’s going on here?? First, we explicitly contemplate the possibility that
keyboard->focus might be NULL and account for it.? Then we fill in a few
other fields, then we repeat the earlier assignment, without the NULL check
and things explode because it’s null.

No relevant comments, so I can’t do much to read the original coder’s mind.
Is this just something that someone replaced and forgot to take out?? I
think that last line should be removed.

SDL mailing list
SDL at lists.libsdl.org
http://lists.libsdl.org/listinfo.cgi/sdl-libsdl.org

–
±----------------------------------------------------------

4

Hi Mason,On Fri, May 14, 2010 at 2:27 AM, Mason Wheeler wrote:

I’ve been tracking down an access violation I get when sending keyboard
input to an SDL window, and I tracked it to SDL_SendKeyboardText in
SDL_keyboard.c.? Specifically, this block:

??? event.text.windowID = keyboard->focus ? keyboard->focus->id : 0;
??? event.text.which = (Uint8) index;
??? SDL_strlcpy(event.text.text, text, SDL_arraysize(event.text.text));
??? event.text.windowID = keyboard->focus->id;

What’s going on here?? First, we explicitly contemplate the possibility that
keyboard->focus might be NULL and account for it.? Then we fill in a few
other fields, then we repeat the earlier assignment, without the NULL check
and things explode because it’s null.

No relevant comments, so I can’t do much to read the original coder’s mind.
Is this just something that someone replaced and forgot to take out?? I
think that last line should be removed.

First of all, you are right, there shouldn’t be another assignment.

Secondly, the current code seems already checked the pointer in the
second assignment [1].

Anyway, the second assignment should be removed.

  • Jiang

[1] http://hg.libsdl.org/SDL/file/fa77a6429698/src/events/SDL_keyboard.c