[PATCH] Windows event handling and SDL_WINDOWID (2 bugs)

I’m trying to convert an existing SDL project as a DLL for a visual
basic (sigh…) application.

I’ve found a bug in SDL_dibevents.c.

When you do a SDL_Quit() and the windows has been inherited with
SDL_WINDOWID the procedure used to handle windows messages remains the
SDL one and doesn’t switch back to the default one, so, if you don’t
close the window the program crashes as soon as you move the mouse
pointer on the window.

I’ve also found another problem in SDL_RegisterApp() in SDL_sysevents.c,
this one occurs if you try to open a window a second time after a
SDL_Quit(), SDL_windowid is a char * pointer to a libc static buffer
that get trashed by any getenv(), this pointer is used anyway quite
often in the application, so I’ve modified SDL_RegisterApp to allocate a
buffer for it and DIB_CreateWindow to check for SDL_WINDOWID changes
every time a window is opened.

This is not an optimal solution, but it makes the whole SDL_WINDOWID
kludge more usable for real world applications.
diff -u -b -r1.1.1.2.2.1 SDL_dibevents.c

— src/video/windib/SDL_dibevents.c 18 Dec 2002 15:37:58 -0000
1.1.1.2.2.1
+++ src/video/windib/SDL_dibevents.c 15 Nov 2004 12:04:47 -0000
@@ -348,7 +348,23 @@
}
ShowWindow(SDL_Window, SW_HIDE);
#else

  • // GG: this call does something only the first time…
    SDL_RegisterApp(“SDL_app”, CS_BYTEALIGNCLIENT, 0);+
  • // GG: so we need to handle the windowid thing also if you close
    and reopen the window
  • if (SDL_windowid) {
  •    char *id_test = getenv("SDL_WINDOWID");
    
  •    if (!id_test || strtol(id_test, NULL, 0) == 0) {
    
  •        free(SDL_windowid);
    
  •        SDL_windowid = NULL;
    
  •    }
    
  •    else if (strcmp(id_test, SDL_windowid)) {
    
  •        free(SDL_windowid);
    
  •        SDL_windowid = strdup(id_test);
    
  •    }
    
  • }
  •    if ( SDL_windowid ) {
               SDL_Window = (HWND)strtol(SDL_windowid, NULL, 0);
    

@@ -379,4 +395,7 @@
if ( SDL_windowid == NULL ) {
DestroyWindow(SDL_Window);
}

  • else if (userWindowProc) {
  •     SetWindowLong(SDL_Window, GWL_WNDPROC, (LONG)userWindowProc);
    
  • }
    }

— src/video/wincommon/SDL_sysevents.c 18 Dec 2002 15:37:58 -0000
1.1.1.2.2.1
+++ src/video/wincommon/SDL_sysevents.c 15 Nov 2004 12:05:36 -0000
@@ -630,7 +630,9 @@
#endif /* WM_MOUSELEAVE */

     /* Check for SDL_WINDOWID hack */
  •   SDL_windowid = getenv("SDL_WINDOWID");
    
  • if (getenv(“SDL_WINDOWID”) != NULL) {

  •   SDL_windowid = strdup(getenv("SDL_WINDOWID"));
    
  • }

    initialized = 1;
    return(0);
    

Bye,
Gabry

When you do a SDL_Quit() and the windows has been inherited with
SDL_WINDOWID the procedure used to handle windows messages remains the
SDL one and doesn’t switch back to the default one, so, if you don’t
close the window the program crashes as soon as you move the mouse
pointer on the window.

I’ve applied your fix to CVS, thanks.

I’ve also found another problem in SDL_RegisterApp() in SDL_sysevents.c,
this one occurs if you try to open a window a second time after a
SDL_Quit(), SDL_windowid is a char * pointer to a libc static buffer
that get trashed by any getenv(), this pointer is used anyway quite
often in the application, so I’ve modified SDL_RegisterApp to allocate a
buffer for it and DIB_CreateWindow to check for SDL_WINDOWID changes
every time a window is opened.

I didn’t see where you free this… Could you update your patch?

Thanks,
-Sam Lantinga, Software Engineer, Blizzard Entertainment

Sam Lantinga wrote:

I’ve also found another problem in SDL_RegisterApp() in SDL_sysevents.c,
this one occurs if you try to open a window a second time after a
SDL_Quit(), SDL_windowid is a char * pointer to a libc static buffer
that get trashed by any getenv(), this pointer is used anyway quite
often in the application, so I’ve modified SDL_RegisterApp to allocate a
buffer for it and DIB_CreateWindow to check for SDL_WINDOWID changes
every time a window is opened.
I didn’t see where you free this… Could you update your patch?

Ok here it is, the only place where it has sense to free it is in the
VideoQuit call. VideoQuit is in another module, so the patch of the
previous post is still valid :slight_smile:

— src/video/windib/SDL_dibvideo.c 18 Dec 2002 15:37:59 -0000
1.1.1.2.2.1
+++ src/video/windib/SDL_dibvideo.c 16 Nov 2004 13:52:19 -0000
@@ -931,6 +931,14 @@
DIB_QuitGamma(this);
DIB_DestroyWindow(this);

  •    /* GG: to handle correctly windowid we had to allocate it (in 
    

SDL_dibevents.c)

  •     * so it should be freed somewhere, and this is the only safe 
    

place to do it

  •     */
    
  •    if (SDL_windowid) {
    
  •        free(SDL_windowid);
    
  •        SDL_windowid = NULL;
    
  •    }+
               SDL_Window = NULL;
       }
    
    }