This is the mail archive of the cygwin-developers@sourceware.cygnus.com mailing list for the Cygwin project.


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]

Re: (patch) makethread stdcall/cdecl confusion


Chris Faylor <cgf@cygnus.com> writes:
> On Thu, Sep 16, 1999 at 10:29:50AM -0500, Mumit Khan wrote:
> >Another issue when you're dealing with thread start routines -- it's 
> >almost always better to malloc the the parameter argument instead of
> >passing the address of a stack data element. It'll work in the current
> >usages in winsup, but this usage can lead to very subtle and hard to
> >track errors. 
> 
> That's probably because, AFAICT, in every case where the argument is
> non-NULL it *is* malloced.

My mistake -- I generalized based on one particular instance, sorry. I
was referring to the parameter passed to thread_stub, which caught my
eye. It is harmless in this particular case since the calling thread 
waits on the callee (synchronization acts as insurance against possible 
stack mismatch).

Oh, the horror stories I can tell about tracking these down ... "it was
a dark and stormy night ... "

How about this:

Thu Sep 16 12:31:46 1999  Mumit Khan  <khan@xraylith.wisc.edu>

	* debug.cc (stdlib.h): Unconditionally include. 
	(thread_stub): Don't use structure copy. Deallocate memory when
	done.
	(makethread): Don't pass stack data to thread start routine.

--- debug.cc.~1	Thu Sep 16 12:14:44 1999
+++ debug.cc	Thu Sep 16 12:29:51 1999
@@ -9,6 +9,7 @@ details. */
 #define NO_DEBUG_DEFINES
 #include "winsup.h"
 #include "exceptions.h"
+#include <stdlib.h>
 
 #undef lock
 #undef unlock
@@ -84,7 +85,7 @@ static DWORD WINAPI
 thread_stub (VOID *arg)
 {
   exception_list except_entry;
-  thread_start info = *((thread_start *) arg);
+  thread_start *info = (thread_start *) arg;
 
   /* marco@ddi.nl: Needed for the reent's  of this local dll thread
      I assume that the local threads are using the reent structure of
@@ -95,11 +96,13 @@ thread_stub (VOID *arg)
 	api_fatal(" Sig proc MT init failed\n");
 #endif
 
-  SetEvent (info.sync);
+  SetEvent (info->sync);
 
   init_exceptions (&except_entry);
 
-  return (*info.func) (info.arg);
+  DWORD retcode = (*info->func) (info->arg);
+  free (info);
+  return retcode;
 }
 
 HANDLE
@@ -109,11 +112,18 @@ makethread (DWORD (*start) (void *), LPV
   DWORD tid;
   HANDLE h;
   SECURITY_ATTRIBUTES *sa;
-  thread_start info;
+  // This is deallocated by thread_stub.
+  thread_start *info = (thread_start *) malloc (sizeof (thread_start));
 
-  info.func = start;
-  info.arg = param;
-  info.sync = CreateEvent (&sec_none_nih, FALSE, FALSE, NULL);
+  if (! info)
+    {
+      debug_printf ("malloc failed, %E");
+      return INVALID_HANDLE_VALUE;
+    }
+
+  info->func = start;
+  info->arg = param;
+  info->sync = CreateEvent (&sec_none_nih, FALSE, FALSE, NULL);
 
   if (*name != '+')
     sa = &sec_none_nih;
@@ -123,12 +133,12 @@ makethread (DWORD (*start) (void *), LPV
       sa = &sec_none;
     }
 
-  if ((h = CreateThread (sa, 0, thread_stub, (VOID *)&info, flags, &tid)))
+  if ((h = CreateThread (sa, 0, thread_stub, (VOID *)info, flags, &tid)))
     {
       regthread (name, tid);
-      WaitForSingleObject (info.sync, INFINITE);
+      WaitForSingleObject (info->sync, INFINITE);
     }
-  CloseHandle (info.sync);
+  CloseHandle (info->sync);
 
   return h;
 }
@@ -164,7 +174,6 @@ threadname (DWORD tid, int lockit)
 }
 
 #ifdef DEBUGGING
-#include <stdlib.h>
 
 typedef struct _h
   {


Regards,
Mumit


Index Nav: [Date Index] [Subject Index] [Author Index] [Thread Index]
Message Nav: [Date Prev] [Date Next] [Thread Prev] [Thread Next]