[Top][All Lists]
[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]
Re: [Bug-gnulib] Module: poll
From: |
Paul Eggert |
Subject: |
Re: [Bug-gnulib] Module: poll |
Date: |
10 Feb 2003 13:02:27 -0800 |
User-agent: |
Gnus/5.0808 (Gnus v5.8.8) Emacs/20.3 |
"Bonzini" <address@hidden> writes:
> Is it ok, or is there anything I should fix in the implementation
> before writing the autoconf tests
Here are some ideas and suggestions.
gnulib <poll.h> should not assume that a working system <poll.h>
merely includes <sys/poll.h>. I would install into gnulib a file
whose name is (say) "poll_.h"; this file should be linked or copied to
"poll.h" by "configure" only if the system doesn't have a working
"poll.h". See fnmatch in gnulib for an example of this. If you do
this, gnulib's poll_.h does not need to include <sys/poll.h>.
Second, the poll emulation should support everything required by the
POSIX standard for poll. See:
http://www.opengroup.org/onlinepubs/007904975/basedefs/poll.h.html
http://www.opengroup.org/onlinepubs/007904975/functions/poll.html
I noticed the following problems when I briefly compared the proposed
poll.h and poll.c to the POSIX standard.
In poll.h:
* nfds_t is not defined. I suggest "typedef unsigned long nfds_t;",
as it's typical.
* 'poll' is not declared.
* The symbol INFTIM is defined, but it shouldn't be present.
* config.h is included, but it isn't needed by poll.h; it should be removed.
In poll.c:
* poll's local variable 'i' should be of type nfds_t.
* poll doesn't fail with errno == EINVAL if nfd exceeds {OPEN_MAX}.
* poll has undefined behavior if one of the file descriptors is
greater than FD_SETSIZE. I'd say it should fail with errno==EOVERFLOW
in that situation (or EINVAL if there is no EOVERFLOW).
* The code "if (maxfd == -1) { errno = EINVAL; return -1; }" doesn't
seem right to me. It's OK to invoke poll with no file descriptors
and with a timeout.
* This code doesn't seem conforming to me. POSIX says that revents
should be 0 in this case.
if (pfd[i].fd < 0) {
pfd[i].revents |= POLLNVAL;
continue;
}
Very minor code improvements:
* You can change "timeout > 0" to "timeout >= 0" and remove the
"timeout == 0" case.
* "ok" can be a boolean, and you can then replace "if (ok) rc++;" with
"rc += ok;".
Also, both poll.c and poll.h say that the files are part of GNU
Smalltalk, but I suppose this part of the copyright notice should be
removed if the files are intended to be generic parts of gnulib.
I'll second Bruno's suggestion to fix the indenting style. Simplest
may be GNU "indent".