From 5f38ec468115a6ddf2c458f2b8f9c4abfd8b0359 Mon Sep 17 00:00:00 2001 From: Christopher Faylor Date: Wed, 23 Nov 2011 21:58:43 +0000 Subject: [PATCH] * pipe.cc (fhandler_pipe::create): Avoid derefencing a NULL pointer. * child_info.h (child_info): Reorganize some elements so that the ones which are initialized in a constructor are all together. * sigproc.cc (child_info::child_info): Initialize values via the constructor rather than as C statements and make sure that flags is set to zero initially. * spawn.cc (child_info_spawn::worker): Use iscygwin() test for determining when to send strace info since it is more foolproof than checking the suspend state. --- winsup/cygwin/ChangeLog | 14 ++++++++++++++ winsup/cygwin/child_info.h | 8 ++++---- winsup/cygwin/pipe.cc | 4 ++-- winsup/cygwin/sigproc.cc | 13 ++++--------- winsup/cygwin/spawn.cc | 2 +- 5 files changed, 25 insertions(+), 16 deletions(-) diff --git a/winsup/cygwin/ChangeLog b/winsup/cygwin/ChangeLog index 6cc089dd9..95ba756d1 100644 --- a/winsup/cygwin/ChangeLog +++ b/winsup/cygwin/ChangeLog @@ -1,3 +1,17 @@ +2011-11-23 Christopher Faylor + + * pipe.cc (fhandler_pipe::create): Avoid derefencing a NULL pointer. + + * child_info.h (child_info): Reorganize some elements so that the ones + which are initialized in a constructor are all together. + * sigproc.cc (child_info::child_info): Initialize values via the + constructor rather than as C statements and make sure that flags is set + to zero initially. + + * spawn.cc (child_info_spawn::worker): Use iscygwin() test for + determining when to send strace info since it is more foolproof than + checking the suspend state. + 2011-11-23 Christopher Faylor * fhandler.h (fhandler_pipe::create): Rename from the misnamed diff --git a/winsup/cygwin/child_info.h b/winsup/cygwin/child_info.h index d69d44166..91cf3c58c 100644 --- a/winsup/cygwin/child_info.h +++ b/winsup/cygwin/child_info.h @@ -57,15 +57,15 @@ public: DWORD intro; // improbable string unsigned long magic; // magic number unique to child_info unsigned short type; // type of record, exec, spawn, fork + init_cygheap *cygheap; + void *cygheap_max; + unsigned char flag; + int retry; // number of times we've tried to start child process HANDLE subproc_ready; // used for synchronization with parent HANDLE user_h; HANDLE parent; - init_cygheap *cygheap; - void *cygheap_max; DWORD cygheap_reserve_sz; - unsigned char flag; unsigned fhandler_union_cb; - int retry; // number of times we've tried to start child process DWORD exit_code; // process exit code static int retry_count;// retry count; child_info (unsigned, child_info_types, bool); diff --git a/winsup/cygwin/pipe.cc b/winsup/cygwin/pipe.cc index 60b59ff62..11ba794b0 100644 --- a/winsup/cygwin/pipe.cc +++ b/winsup/cygwin/pipe.cc @@ -264,14 +264,14 @@ fhandler_pipe::create (LPSECURITY_ATTRIBUTES sa_ptr, PHANDLE r, PHANDLE w, /* The pipe is already open with compatible parameters. Pick a new name and retry. */ debug_printf ("pipe busy", !name ? ", retrying" : ""); - if (!*name) + if (!name) *r = NULL; break; case ERROR_ACCESS_DENIED: /* The pipe is already open with incompatible parameters. Pick a new name and retry. */ debug_printf ("pipe access denied%s", !name ? ", retrying" : ""); - if (!*name) + if (!name) *r = NULL; break; default: diff --git a/winsup/cygwin/sigproc.cc b/winsup/cygwin/sigproc.cc index 43b329373..bbfed1fcd 100644 --- a/winsup/cygwin/sigproc.cc +++ b/winsup/cygwin/sigproc.cc @@ -761,10 +761,11 @@ int child_info::retry_count = 10; /* Initialize some of the memory block passed to child processes by fork/spawn/exec. */ child_info::child_info (unsigned in_cb, child_info_types chtype, - bool need_subproc_ready) + bool need_subproc_ready): + cb (in_cb), intro (PROC_MAGIC_GENERIC), magic (CHILD_INFO_MAGIC), + type (chtype), cygheap (::cygheap), cygheap_max (::cygheap_max), + flag (0), retry (child_info::retry_count) { - cb = in_cb; - /* It appears that when running under WOW64 on Vista 64, the first DWORD value in the datastructure lpReserved2 is pointing to (msv_count in Cygwin), has to reflect the size of that datastructure as used in the @@ -790,9 +791,6 @@ child_info::child_info (unsigned in_cb, child_info_types chtype, datastructure. */ msv_count = wincap.needs_count_in_si_lpres2 () ? in_cb / 5 : 0; - intro = PROC_MAGIC_GENERIC; - magic = CHILD_INFO_MAGIC; - type = chtype; fhandler_union_cb = sizeof (fhandler_union); user_h = cygwin_user_h; if (strace.attached ()) @@ -803,9 +801,6 @@ child_info::child_info (unsigned in_cb, child_info_types chtype, flag |= _CI_ISCYGWIN; } sigproc_printf ("subproc_ready %p", subproc_ready); - cygheap = ::cygheap; - cygheap_max = ::cygheap_max; - retry = child_info::retry_count; /* Create an inheritable handle to pass to the child process. This will allow the child to duplicate handles from the parent to itself. */ parent = NULL; diff --git a/winsup/cygwin/spawn.cc b/winsup/cygwin/spawn.cc index 25072a886..4e93b1888 100644 --- a/winsup/cygwin/spawn.cc +++ b/winsup/cygwin/spawn.cc @@ -692,7 +692,7 @@ loop: goto out; } - if (!(c_flags & CREATE_SUSPENDED)) + if (iscygwin ()) strace.write_childpid (*this, pi.dwProcessId); /* Fixup the parent data structures if needed and resume the child's