This is the mail archive of the cygwin-patches 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]
Other format: [Raw text]

Re: [PATCH] Allow usage of union wait for wait() functions and macros


Christopher Faylor wrote:
On Wed, Oct 05, 2011 at 12:57:44PM +0200, Christian Franke wrote:
...
diff --git a/winsup/cygwin/include/cygwin/wait.h b/winsup/cygwin/include/cygwin/wait.h
index bed81b7..e4edba2 100644
--- a/winsup/cygwin/include/cygwin/wait.h
+++ b/winsup/cygwin/include/cygwin/wait.h
@@ -1,6 +1,6 @@
/* cygwin/wait.h

-   Copyright 2006, 2009 Red Hat, Inc.
+   Copyright 2006, 2009, 2011 Red Hat, Inc.

This file is part of Cygwin.

@@ -16,6 +16,9 @@ details. */
#define WCONTINUED 8
#define __W_CONTINUED	0xffff

+/* Will be redefined in sys/wait.h.  */
+#define __wait_status_to_int(w)  (w)
+
Why is this necessary? It doesn't look like it is ever expanded in cygwin/wait.h.

This would be needed if cygwin/wait.h is included separately without sys/wait.h
(e.g. stdlib.h -> cygwin/stdlib.h -> cygwin/wait.h)
and some W*() macro is actually used.



If a redefinition is necessary why not put it all in one place?

The W*() macros and union wait are closely related. So a probably better approach would be to move union wait to cygwin/wait.h and define __wait_status_to_int() only there.


But then C++ compile may fail because cygwin/wait.h is sometimes included indirectly inside an extern "C" block:
w32api/shlobj.h -> extern "C" { w32api/ole2.h ...-> stdlib.h ...-> cygwin/wait.h }



And why is redefinition needed inside Cygwin?

It is not redefined in the __INSIDE_CYGWIN__ case.



...
+
+#endif
Could you add a comment here and at the #else indicating what they refer to
like #else /* !(defined(__cplusplus) || defined(__INSIDE_CYGWIN__)) and
#endif (defined(__cplusplus) || defined(__INSIDE_CYGWIN__) ?

OK.



Also since Cygwin is C++ why do you need the __INSIDE_CYGWIN__ here?

There are still ten *.c files in winsup/cygwin :-)


New patch attached. Also includes the comment fix suggested by Eric Blake.

Christian

2011-10-05  Christian Franke  <franke@computer.org>

	* include/cygwin/wait.h: Use new __wait_status_to_int()
	macro to access status value in W*() status checks.
	Fix status description.
	* include/sys/wait.h: Allow `int' and `union wait' as
	wait status parameter.  Change __wait_status_to_int()
	macro and wait () prototypes accordingly.  Add inline
	functions for C++.  Remove extra `;'.

diff --git a/winsup/cygwin/include/cygwin/wait.h b/winsup/cygwin/include/cygwin/wait.h
index bed81b7..0f3f763 100644
--- a/winsup/cygwin/include/cygwin/wait.h
+++ b/winsup/cygwin/include/cygwin/wait.h
@@ -1,6 +1,6 @@
 /* cygwin/wait.h
 
-   Copyright 2006, 2009 Red Hat, Inc.
+   Copyright 2006, 2009, 2011 Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -16,8 +16,11 @@ details. */
 #define WCONTINUED 8
 #define __W_CONTINUED	0xffff
 
-/* A status looks like:
-      <2 bytes info> <2 bytes code>
+/* Will be redefined in sys/wait.h.  */
+#define __wait_status_to_int(w)  (w)
+
+/* A status is 16 bits, and looks like:
+      <1 byte info> <1 byte code>
 
       <code> == 0, child has exited, info is the exit value
       <code> == 1..7e, child has exited, info is the signal number.
@@ -25,13 +28,14 @@ details. */
       <code> == 80, there was a core dump.
 */
 
-#define WIFEXITED(w)	(((w) & 0xff) == 0)
-#define WIFSIGNALED(w)	(((w) & 0x7f) > 0 && (((w) & 0x7f) < 0x7f))
-#define WIFSTOPPED(w)	(((w) & 0xff) == 0x7f)
-#define WIFCONTINUED(w)	(((w) & 0xffff) == __W_CONTINUED)
-#define WEXITSTATUS(w)	(((w) >> 8) & 0xff)
-#define WTERMSIG(w)	((w) & 0x7f)
+#define WIFEXITED(w)	((__wait_status_to_int(w) & 0xff) == 0)
+#define WIFSIGNALED(w)	((__wait_status_to_int(w) & 0x7f) > 0 \
+			 && ((__wait_status_to_int(w) & 0x7f) < 0x7f))
+#define WIFSTOPPED(w)	((__wait_status_to_int(w) & 0xff) == 0x7f)
+#define WIFCONTINUED(w)	((__wait_status_to_int(w) & 0xffff) == __W_CONTINUED)
+#define WEXITSTATUS(w)	((__wait_status_to_int(w) >> 8) & 0xff)
+#define WTERMSIG(w)	(__wait_status_to_int(w) & 0x7f)
 #define WSTOPSIG	WEXITSTATUS
-#define WCOREDUMP(w)	(WIFSIGNALED(w) && (w & 0x80))
+#define WCOREDUMP(w)	(WIFSIGNALED(w) && (__wait_status_to_int(w) & 0x80))
 
 #endif /* _CYGWIN_WAIT_H */
diff --git a/winsup/cygwin/include/sys/wait.h b/winsup/cygwin/include/sys/wait.h
index 04bbae7..811417e 100644
--- a/winsup/cygwin/include/sys/wait.h
+++ b/winsup/cygwin/include/sys/wait.h
@@ -1,6 +1,6 @@
 /* sys/wait.h
 
-   Copyright 1997, 1998, 2001, 2002, 2003, 2004, 2006 Red Hat, Inc.
+   Copyright 1997, 1998, 2001, 2002, 2003, 2004, 2006, 2011 Red Hat, Inc.
 
 This file is part of Cygwin.
 
@@ -19,10 +19,25 @@ details. */
 extern "C" {
 #endif
 
-pid_t wait (int *);
-pid_t waitpid (pid_t, int *, int);
-pid_t wait3 (int *__status, int __options, struct rusage *__rusage);
-pid_t wait4 (pid_t __pid, int *__status, int __options, struct rusage *__rusage);
+#if defined(__cplusplus) || defined(__INSIDE_CYGWIN__)
+
+typedef int *__wait_status_ptr_t;
+
+#else /* !(defined(__cplusplus) || defined(__INSIDE_CYGWIN__)) */
+
+/* Allow `int' and `union wait' for the status.  */
+typedef union
+  {
+    int *__int_ptr;
+    union wait *__union_wait_ptr;
+  } __wait_status_ptr_t  __attribute__ ((__transparent_union__));
+
+#endif /* defined(__cplusplus) || defined(__INSIDE_CYGWIN__) */
+
+pid_t wait (__wait_status_ptr_t __status);
+pid_t waitpid (pid_t __pid, __wait_status_ptr_t __status, int __options);
+pid_t wait3 (__wait_status_ptr_t __status, int __options, struct rusage *__rusage);
+pid_t wait4 (pid_t __pid, __wait_status_ptr_t __status, int __options, struct rusage *__rusage);
 
 union wait
   {
@@ -49,7 +64,37 @@ union wait
 #define	w_stopval	__wait_stopped.__w_stopval
 
 #ifdef __cplusplus
-};
+}
 #endif
 
-#endif
+#ifndef __INSIDE_CYGWIN__
+
+/* Used in cygwin/wait.h, redefine to accept `int' and `union wait'.  */
+#undef __wait_status_to_int
+
+#ifdef __cplusplus
+
+inline int __wait_status_to_int (int __status)
+  { return __status; }
+inline int __wait_status_to_int (const union wait & __status)
+  { return __status.w_status; }
+
+/* C++ wait() variants for `union wait'.  */
+inline pid_t wait (union wait *__status)
+  { return wait ((int *) __status); }
+inline pid_t waitpid (pid_t __pid, union wait *__status, int __options)
+  { return waitpid(__pid, (int *) __status, __options); }
+inline pid_t wait3 (union wait *__status, int __options, struct rusage *__rusage)
+  { return wait3 ((int *) __status, __options, __rusage); }
+inline pid_t wait4 (pid_t __pid, union wait *__status, int __options, struct rusage *__rusage)
+  { return wait4 (__pid, (int *) __status, __options, __rusage); }
+
+#else /* !__cplusplus */
+
+#define __wait_status_to_int(__status)  (__extension__ \
+  (((union { __typeof(__status) __in; int __out; }) { .__in = (__status) }).__out))
+
+#endif /* __cplusplus */
+#endif /* !__INSIDE_CYGWIN__ */
+
+#endif /* _SYS_WAIT_H */


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