Re: select.2: update includes
On Tue, Dec 3, 2013 at 12:08 PM, Christian Weisgerber na...@mips.inka.de wrote: ok? ok matthew +.Fd #include sys/select.h Worth using .In instead while you're at it?
Re: select.2: update includes
On Tue, Dec 3, 2013 at 12:08 PM, Christian Weisgerber na...@mips.inka.de wrote: POSIX says select() and everything it needs should come from sys/select.h. Eight years ago our headers have been fixed to provide this. Time to acknowledge this in the man page, too. ok? ... -.Fd #include string.h string.h needs to stay until FD_ZERO() and FD_COPY() are changed to not use memset()/memcpy(). Philip
Re: select.2: update includes
Philip Guenther:
string.h needs to stay until FD_ZERO() and FD_COPY() are changed to
not use memset()/memcpy().
We could grab this from FreeBSD:
#if __BSD_VISIBLE
#define FD_COPY(f, t) (void)(*(t) = *(f))
#endif
#define FD_ZERO(p) do { \
fd_set *_p; \
__size_t _n;\
\
_p = (p); \
_n = __howmany(FD_SETSIZE, __NFDBITS); \
while (_n 0) \
_p-fds_bits[--_n] = 0; \
} while (0)
--
Christian naddy Weisgerber na...@mips.inka.de
Re: select.2: update includes
On Tue, Dec 3, 2013 at 12:37 PM, Philip Guenther guent...@gmail.com wrote: string.h needs to stay until FD_ZERO() and FD_COPY() are changed to not use memset()/memcpy(). Good point. Would something like this work? #define FD_COPY(f, t) (*(fd_set *)(t) = *(const fd_set *)(f)) static const fd_set __fd_zero_set; #define FD_ZERO(p) FD_COPY(__fd_zero_set, p) Downside is we lose const warnings. We could get those back if we change FD_COPY into an inline function instead of a macro, or do some GNU block-expression trickery. Or we can just declare memset/memcpy in sys/select.h too?
Re: select.2: update includes
On Tue, Dec 3, 2013 at 1:03 PM, Matthew Dempsky matt...@dempsky.org wrote: On Tue, Dec 3, 2013 at 12:37 PM, Philip Guenther guent...@gmail.com wrote: string.h needs to stay until FD_ZERO() and FD_COPY() are changed to not use memset()/memcpy(). Good point. Would something like this work? #define FD_COPY(f, t) (*(fd_set *)(t) = *(const fd_set *)(f)) What problem does the casts solve? (And actually, as noted in naddy's cite from FreeBSD, FD_COPY() is a non-standard BSDism, so it can just continue to assume the caller pulled in string.h) static const fd_set __fd_zero_set; #define FD_ZERO(p) FD_COPY(__fd_zero_set, p) That'll put a __fd_zero_set in each .o that references FD_ZERO(), ick. I think I would prefer either the FreeBSD inline bzero() routine (although zeroing from the end seems an odd choice for caching reasons, no?), or adding a strong alias to libc, _memset == memset, and then having sys/select.h say void *_memset(void *, int, size_t); #define FD_ZERO(p) (void )_memset(p, 0, sizeof(*(p))) Or we can just declare memset/memcpy in sys/select.h too? Magic POSIX 8-ball says no. Philip Guenther
Re: select.2: update includes
On Tue, Dec 3, 2013 at 12:37 PM, Philip Guenther guent...@gmail.com wrote: string.h needs to stay until FD_ZERO() and FD_COPY() are changed to not use memset()/memcpy(). Good point. Would something like this work? #define FD_COPY(f, t) (*(fd_set *)(t) = *(const fd_set *)(f)) Regarding your FD_ZERO: static const fd_set __fd_zero_set; #define FD_ZERO(p) FD_COPY(__fd_zero_set, p) I'm not super happy about that well-located gigantic .rodata nop sled. Furhermore, as const, it will be represented in the actual filesize of every executable. Downside is we lose const warnings. We could get those back if we change FD_COPY into an inline function instead of a macro, or do some GNU block-expression trickery. Hmm. Second reason against it. I think what naddy showed seems better, though the FD_ZERO expression can do with a bit more packing.
Re: select.2: update includes
Combining the various suggestions, I now have this:
Index: sys/sys/select.h
===
RCS file: /cvs/src/sys/sys/select.h,v
retrieving revision 1.13
diff -u -p -r1.13 select.h
--- sys/sys/select.h29 Oct 2013 02:44:52 - 1.13
+++ sys/sys/select.h3 Dec 2013 21:51:08 -
@@ -76,8 +76,17 @@ typedef struct fd_set {
((p)-fds_bits[(n) / __NFDBITS] = ~(1U ((n) % __NFDBITS)))
#defineFD_ISSET(n, p) \
((p)-fds_bits[(n) / __NFDBITS] (1U ((n) % __NFDBITS)))
-#defineFD_COPY(f, t) memcpy(t, f, sizeof(*(f)))
-#defineFD_ZERO(p) memset(p, 0, sizeof(*(p)))
+
+#if __BSD_VISIBLE
+#defineFD_COPY(f, t) (void)(*(t) = *(f))
+#endif
+#defineFD_ZERO(p) do \
+ fd_set *_p = (p); \
+ __size_t _n = __howmany(FD_SETSIZE, __NFDBITS); \
+ \
+ while (_n 0) \
+ _p-fds_bits[--_n] = 0; \
+} while (0)
#if __BSD_VISIBLE
#defineNBBY__NBBY
Index: lib/libc/sys/select.2
===
RCS file: /cvs/src/lib/libc/sys/select.2,v
retrieving revision 1.32
diff -u -p -r1.32 select.2
--- lib/libc/sys/select.2 2 Nov 2013 17:25:34 - 1.32
+++ lib/libc/sys/select.2 3 Dec 2013 21:43:17 -
@@ -38,10 +38,7 @@
.Nm pselect
.Nd synchronous I/O multiplexing
.Sh SYNOPSIS
-.Fd #include sys/types.h
-.Fd #include sys/time.h
-.Fd #include string.h
-.Fd #include unistd.h
+.In sys/select.h
.Ft int
.Fn select int nfds fd_set *readfds fd_set *writefds fd_set *exceptfds
struct timeval *timeout
.Ft int
--
Christian naddy Weisgerber na...@mips.inka.de
Re: select.2: update includes
On Tue, Dec 3, 2013 at 1:55 PM, Christian Weisgerber na...@mips.inka.de wrote:
+#if __BSD_VISIBLE
+#defineFD_COPY(f, t) (void)(*(t) = *(f))
+#endif
+#defineFD_ZERO(p) do \
+ fd_set *_p = (p); \
+ __size_t _n = __howmany(FD_SETSIZE, __NFDBITS); \
+ \
+ while (_n 0) \
+ _p-fds_bits[--_n] = 0; \
+} while (0)
I think you're missing a { after the do. :)
But otherwise looks ok to me.
Re: select.2: update includes
On Tue, Dec 3, 2013 at 1:39 PM, Philip Guenther guent...@gmail.com wrote: What problem does the casts solve? I wasn't sure if people might be calling FD_COPY()/FD_ZERO() with void* or char* typed arguments (or other custom sized types). If we can assume they'll only pass fd_set* arguments, then they could be done away with. (And actually, as noted in naddy's cite from FreeBSD, FD_COPY() is a non-standard BSDism, so it can just continue to assume the caller pulled in string.h) Sounds good to me. That'll put a __fd_zero_set in each .o that references FD_ZERO(), ick. Oops, yeah, I was thinking they'd be collapsed into a single const value in the output object, but I forgot they'd need to be kept separate for object identity.
Re: select.2: update includes
I wasn't sure if people might be calling FD_COPY()/FD_ZERO() with void* or char* typed arguments (or other custom sized types). If we can assume they'll only pass fd_set* arguments, then they could be done away with. Hmm, that's a good question. The base appears clean, but it rarely makes promises for the rest of the ecosystem.
Re: select.2: update includes
Matthew Dempsky:
I think you're missing a { after the do. :)
Hmm, yes. This survives a make build:
Index: sys/sys/select.h
===
RCS file: /cvs/src/sys/sys/select.h,v
retrieving revision 1.13
diff -u -p -r1.13 select.h
--- sys/sys/select.h29 Oct 2013 02:44:52 - 1.13
+++ sys/sys/select.h3 Dec 2013 22:08:31 -
@@ -76,8 +76,17 @@ typedef struct fd_set {
((p)-fds_bits[(n) / __NFDBITS] = ~(1U ((n) % __NFDBITS)))
#defineFD_ISSET(n, p) \
((p)-fds_bits[(n) / __NFDBITS] (1U ((n) % __NFDBITS)))
-#defineFD_COPY(f, t) memcpy(t, f, sizeof(*(f)))
-#defineFD_ZERO(p) memset(p, 0, sizeof(*(p)))
+
+#if __BSD_VISIBLE
+#defineFD_COPY(f, t) (void)(*(t) = *(f))
+#endif
+#defineFD_ZERO(p) do { \
+ fd_set *_p = (p); \
+ __size_t _n = __howmany(FD_SETSIZE, __NFDBITS); \
+ \
+ while (_n 0) \
+ _p-fds_bits[--_n] = 0; \
+} while (0)
#if __BSD_VISIBLE
#defineNBBY__NBBY
Index: lib/libc/sys/select.2
===
RCS file: /cvs/src/lib/libc/sys/select.2,v
retrieving revision 1.32
diff -u -p -r1.32 select.2
--- lib/libc/sys/select.2 2 Nov 2013 17:25:34 - 1.32
+++ lib/libc/sys/select.2 3 Dec 2013 21:43:17 -
@@ -38,10 +38,7 @@
.Nm pselect
.Nd synchronous I/O multiplexing
.Sh SYNOPSIS
-.Fd #include sys/types.h
-.Fd #include sys/time.h
-.Fd #include string.h
-.Fd #include unistd.h
+.In sys/select.h
.Ft int
.Fn select int nfds fd_set *readfds fd_set *writefds fd_set *exceptfds
struct timeval *timeout
.Ft int
--
Christian naddy Weisgerber na...@mips.inka.de
