[PATCH] sys/types.h: Avoid bit-manipulation of signed values

Corinna Vinschen vinschen@redhat.com
Mon Nov 16 16:28:00 GMT 2015


On Nov 16 11:52, Sebastian Huber wrote:
> On 14/11/15 10:55, Corinna Vinschen wrote:
> >On Nov 13 15:44, Sebastian Huber wrote:
> >>>On 13/11/15 10:53, Corinna Vinschen wrote:
> >>>> >RTEMS guys, any input on how you handle this stuff?  Do we have to stick
> >>>> >to sys/types.h or shall we carefully rearrange the definitions to be
> >>>> >better aligned with BSD, Linux, et al?
> >>>
> >>>It would be good to rearrange the definitions to be better aligned with BSD
> >>>and Linux. My long term goal is to get rid of the RTEMS-specific param.h
> >>>(newlib/libc/sys/rtems/include/sys/param.h).
> >Sounds good.  What about sys/select.h and the matching macros in
> >sys/types.h?
> 
> Newlib has currently no sys/select.h. RTEMS provides its own copy of this
> file. Do you plan to import a sys/select.h into Newlib and use it for
> Cygwin?

Cygwin is using its own copy of sys/select.h.  I hacked a bit on this
and I came up with a sys/select.h which works for Cygwin and which might
be ok for inclusion into newlib.  See the attached patch.  I removed the
source patches required to make this work on Cygwin, so this patch only
shows what affects all targets.

Note especially:

- I moved NBBY to sys/param.h.

- Define howmany in sys/param.h.

In sys/select.h, as on FreeBSD:

- Use "8" instead of "NBBY" (avoids dependency to sys/param.h).

- Redefine fd_mask as unsigned long, rather than signed long.

- Use _howmany instead of howmany.

Please have a look if these changes would be ok for you.


Thanks,
Corinna

-- 
Corinna Vinschen
Cygwin Maintainer
Red Hat
-------------- next part --------------
diff --git a/newlib/libc/include/sys/param.h b/newlib/libc/include/sys/param.h
index ef203d3..9a6f115 100644
--- a/newlib/libc/include/sys/param.h
+++ b/newlib/libc/include/sys/param.h
@@ -10,6 +10,9 @@
 #include <machine/endian.h>
 #include <machine/param.h>
 
+#ifndef NBBY
+# define NBBY 8		/* number of bits in a byte */
+#endif
 #ifndef HZ
 # define HZ (60)
 #endif
@@ -25,4 +28,8 @@
 #define MAX(a,b) ((a) > (b) ? (a) : (b))
 #define MIN(a,b) ((a) < (b) ? (a) : (b))
 
+#ifndef howmany
+#define    howmany(x, y)   (((x)+((y)-1))/(y))
+#endif
+
 #endif
diff --git a/newlib/libc/include/sys/types.h b/newlib/libc/include/sys/types.h
index d8d6fdc..5dd6c75 100644
--- a/newlib/libc/include/sys/types.h
+++ b/newlib/libc/include/sys/types.h
@@ -208,52 +208,6 @@ typedef unsigned int mode_t _ST_INT32;
 
 typedef unsigned short nlink_t;
 
-/* We don't define fd_set and friends if we are compiling POSIX
-   source, or if we have included (or may include as indicated
-   by __USE_W32_SOCKETS) the W32api winsock[2].h header which
-   defines Windows versions of them.   Note that a program which
-   includes the W32api winsock[2].h header must know what it is doing;
-   it must not call the cygwin32 select function.
-*/
-# if !(defined (_POSIX_SOURCE) || defined (_WINSOCK_H) || defined (_WINSOCKAPI_) || defined (__USE_W32_SOCKETS)) 
-#  define _SYS_TYPES_FD_SET
-#  define	NBBY	8		/* number of bits in a byte */
-/*
- * Select uses bit masks of file descriptors in longs.
- * These macros manipulate such bit fields (the filesystem macros use chars).
- * FD_SETSIZE may be defined by the user, but the default here
- * should be >= NOFILE (param.h).
- */
-#  ifndef	FD_SETSIZE
-#	define	FD_SETSIZE	64
-#  endif
-
-typedef	long	fd_mask;
-#  define	NFDBITS	(sizeof (fd_mask) * NBBY)	/* bits per mask */
-#  ifndef	howmany
-#	define	howmany(x,y)	(((x)+((y)-1))/(y))
-#  endif
-
-/* We use a macro for fd_set so that including Sockets.h afterwards
-   can work.  */
-typedef	struct _types_fd_set {
-	fd_mask	fds_bits[howmany(FD_SETSIZE, NFDBITS)];
-} _types_fd_set;
-
-#define fd_set _types_fd_set
-
-#  define	FD_SET(n, p)	((p)->fds_bits[(n)/NFDBITS] |= (1L << ((n) % NFDBITS)))
-#  define	FD_CLR(n, p)	((p)->fds_bits[(n)/NFDBITS] &= ~(1L << ((n) % NFDBITS)))
-#  define	FD_ISSET(n, p)	((p)->fds_bits[(n)/NFDBITS] & (1L << ((n) % NFDBITS)))
-#  define	FD_ZERO(p)	(__extension__ (void)({ \
-     size_t __i; \
-     char *__tmp = (char *)p; \
-     for (__i = 0; __i < sizeof (*(p)); ++__i) \
-       *__tmp++ = 0; \
-}))
-
-# endif	/* !(defined (_POSIX_SOURCE) || defined (_WINSOCK_H) || defined (_WINSOCKAPI_) || defined (__USE_W32_SOCKETS)) */
-
 #undef __MS_types__
 #undef _ST_INT32
 
diff --git a/winsup/cygwin/include/sys/select.h b/winsup/cygwin/include/sys/select.h
index 9cc6c1e..ffe8900 100644
--- a/winsup/cygwin/include/sys/select.h
+++ b/winsup/cygwin/include/sys/select.h
@@ -12,7 +12,14 @@ details. */
 #ifndef _SYS_SELECT_H
 #define _SYS_SELECT_H
 
-#if !defined (_POSIX_SOURCE) && !defined (__INSIDE_CYGWIN_NET__) && !defined (__USE_W32_SOCKETS)
+/* We don't define fd_set and friends if we are compiling POSIX
+   source, or if we have included (or may include as indicated
+   by __USE_W32_SOCKETS) the W32api winsock[2].h header which
+   defines Windows versions of them.   Note that a program which
+   includes the W32api winsock[2].h header must know what it is doing;
+   it must not call the Cygwin select function.
+*/
+# if !(defined (_POSIX_SOURCE) || defined (_WINSOCK_H) || defined (_WINSOCKAPI_) || defined (__USE_W32_SOCKETS))
 
 #include <sys/cdefs.h>
 
@@ -26,6 +33,43 @@ details. */
 /* Get definition of sigset_t. */
 #include <signal.h>
 
+#  define _SYS_TYPES_FD_SET
+/*
+ * Select uses bit masks of file descriptors in longs.
+ * These macros manipulate such bit fields (the filesystem macros use chars).
+ * FD_SETSIZE may be defined by the user, but the default here
+ * should be >= NOFILE (param.h).
+ */
+#  ifndef	FD_SETSIZE
+#	define	FD_SETSIZE	64
+#  endif
+
+typedef	unsigned long	fd_mask;
+#  define	NFDBITS	(sizeof (fd_mask) * 8)	/* bits per mask */
+#  ifndef	_howmany
+#	define	_howmany(x,y)	(((x)+((y)-1))/(y))
+#  endif
+
+/* We use a macro for fd_set so that including Sockets.h afterwards
+   can work.  */
+typedef	struct _types_fd_set {
+	fd_mask	fds_bits[_howmany(FD_SETSIZE, NFDBITS)];
+} _types_fd_set;
+
+#define fd_set _types_fd_set
+
+#  define	FD_SET(n, p)	((p)->fds_bits[(n)/NFDBITS] |= (1L << ((n) % NFDBITS)))
+#  define	FD_CLR(n, p)	((p)->fds_bits[(n)/NFDBITS] &= ~(1L << ((n) % NFDBITS)))
+#  define	FD_ISSET(n, p)	((p)->fds_bits[(n)/NFDBITS] & (1L << ((n) % NFDBITS)))
+#  define	FD_ZERO(p)	(__extension__ (void)({ \
+     size_t __i; \
+     char *__tmp = (char *)p; \
+     for (__i = 0; __i < sizeof (*(p)); ++__i) \
+       *__tmp++ = 0; \
+}))
+
+#if !defined (__INSIDE_CYGWIN_NET__)
+
 __BEGIN_DECLS
 
 int select __P ((int __n, fd_set *__readfds, fd_set *__writefds,
@@ -36,6 +80,8 @@ int pselect __P ((int __n, fd_set *__readfds, fd_set *__writefds,
 
 __END_DECLS
 
+#endif
+
 #endif /* !_POSIX_SOURCE, !__INSIDE_CYGWIN_NET__ */
 
 #endif /* sys/select.h */
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 819 bytes
Desc: not available
URL: <http://sourceware.org/pipermail/newlib/attachments/20151116/d68b6fbf/attachment.sig>


More information about the Newlib mailing list