Skip to content

Commit

Permalink
Try to ensure all calls return an error code.
Browse files Browse the repository at this point in the history
(some used to return the fd, some used to return -1, etc...)
  • Loading branch information
Scott Bronson committed Mar 29, 2007
1 parent 6eeab98 commit cc8fe6b
Show file tree
Hide file tree
Showing 7 changed files with 58 additions and 47 deletions.
6 changes: 4 additions & 2 deletions CHANGES
Expand Up @@ -6,7 +6,7 @@ TODO
- Rename poller.h to io.h -- it's the .h file that should be included by everything.
- The way you select a poller now (-DUSE_EPOLL, -DUSE_POLL, -DUSE_SELECT) is wack.
Figure out how to sniff compiler #defines to figure out what pollers are available.
- Normalize errors returned from calls.
- Normalize errors returned from calls. Ensure all pollers return well-defined, predictable errors.
- iotest and socktest need to print an error if write data was discarded.
- Fix the io_parse_address routine -- its return value currently sucks rocks.
- Pass MAXFDS and MAXEVENTS to io_init. That should normalize all the maxfd junk lying around.
Expand All @@ -17,9 +17,11 @@ TODO
- TODO TODO add unit tests (maybe) and lots of functional tests (definitely!)
- Make socket.c IPv6 aware, http://people.redhat.com/drepper/userapi-ipv6.html
Of course... probably want to leave the ipv4 hooks around too, since not everything is ipv6 yet?

- io_wait is now the only call that doesn't return an error code... (it returns the number of fds containing events) Is this OK?

DONE:
* Try to standardize return codes: every one returns an error code or 0 on no error.
(except for io_wait, which still returns the # of events that need to be dispatched).
* Added io_close. This allows mock fds to close without closing, say, stdout.
* Made io_socket_connect_fd private again. To replace its usage, create an automatic
io_atom on the stack, call io_socket_connect with that atom, then copy the atom
Expand Down
14 changes: 8 additions & 6 deletions atom.c
Expand Up @@ -72,11 +72,10 @@ int io_atom_read(struct io_poller *poller, io_atom *io, char *buf, size_t cnt, s
*
* @returns 0 on success, the error code if there was a failure.
*
* A return value of -EPIPE means that the remote peer closed its
* connection. HOWEVER, your app will already have received a
* SIGPIPE, so you'll have to block this signal if you want to
* handle the -EPIPE return code (probably a very good idea).
*
* A return value of EPIPE means that the remote peer closed its
* connection. If it's a real EPIPE, your app will already have
* received a SIGPIPE.
*
* We treat handle the remote resetting the connection the same as
* it closing the connection. So, when errno is ECONNRESET, this
* routine will return EPIPE to tell you that the remote is gone.
Expand Down Expand Up @@ -120,7 +119,10 @@ int io_atom_close(struct io_poller *poller, io_atom *io)
io_remove(poller, io);
fd = io->fd;
io->fd = -1;
return close(fd);
if(close(fd)) {
return errno ? errno : -1;
}
return 0;
}


Expand Down
20 changes: 16 additions & 4 deletions pollers/epoll.c
Expand Up @@ -50,7 +50,10 @@ int io_epoll_init(io_epoll_poller *poller)

int io_epoll_poller_dispose(io_epoll_poller *poller)
{
return close(poller->epfd);
if(close(poller->epfd)) {
return errno ? errno : -1;
}
return 0;
}


Expand Down Expand Up @@ -82,7 +85,10 @@ int io_epoll_add(io_epoll_poller *poller, io_atom *atom, int flags)
struct epoll_event event;
event.data.ptr = atom;
event.events = get_events(flags);
return epoll_ctl(poller->epfd, EPOLL_CTL_ADD, atom->fd, &event);
if(epoll_ctl(poller->epfd, EPOLL_CTL_ADD, atom->fd, &event)) {
return errno ? errno : -1;
}
return 0;
}


Expand All @@ -91,15 +97,21 @@ int io_epoll_set(io_epoll_poller *poller, io_atom *atom, int flags)
struct epoll_event event;
event.data.ptr = atom;
event.events = get_events(flags);
return epoll_ctl(poller->epfd, EPOLL_CTL_MOD, atom->fd, &event);
if(epoll_ctl(poller->epfd, EPOLL_CTL_MOD, atom->fd, &event)) {
return errno ? errno : -1;
}
return 0;
}


int io_epoll_remove(io_epoll_poller *poller, io_atom *atom)
{
// Linux has been able to handle a NULL event only since 2.6.9.
struct epoll_event event;
return epoll_ctl(poller->epfd, EPOLL_CTL_DEL, atom->fd, &event);
if(epoll_ctl(poller->epfd, EPOLL_CTL_DEL, atom->fd, &event)) {
return errno ? errno : -1;
}
return 0;
}


Expand Down
33 changes: 18 additions & 15 deletions pollers/poll.c
Expand Up @@ -85,13 +85,13 @@ int io_poll_add(io_poll_poller *poller, io_atom *atom, int flags)
int index, avail_fd;

if(atom->fd < 0) {
return -ERANGE;
return ERANGE;
}

index = find_fd(poller, atom->fd, &avail_fd);
if(index >= 0) {
// this fd is already being monitored!
return -EALREADY;
return EALREADY;
}

if(avail_fd == -1) {
Expand All @@ -111,28 +111,30 @@ int io_poll_add(io_poll_poller *poller, io_atom *atom, int flags)
}


static int poll_find(io_poll_poller *poller, int fd)
{
int index;
static int poll_find(io_poll_poller *poller, int fd, int *index)
{
int err;

if(fd < 0) {
return -ERANGE;
return ERANGE;
}

index = find_fd(poller, fd, NULL);
err = find_fd(poller, fd, NULL);
if(index < 0) {
return -EEXIST;
return EEXIST;
}

return index;
*index = err;
return 0;
}


int io_poll_set(io_poll_poller *poller, io_atom *atom, int flags)
{
int index = poll_find(poller, atom->fd);
if(index < 0) {
return index;
int index;
int err = poll_find(poller, atom->fd, &index);
if(err) {
return err;
}

poller->pfds[index].events = get_events(flags);
Expand All @@ -142,9 +144,10 @@ int io_poll_set(io_poll_poller *poller, io_atom *atom, int flags)

int io_poll_remove(io_poll_poller *poller, io_atom *atom)
{
int index = poll_find(poller, atom->fd);
if(index < 0) {
return index;
int index;
int err = poll_find(poller, atom->fd, &index);
if(err) {
return err;
}

poller->pfds[index].fd = -1;
Expand Down
12 changes: 6 additions & 6 deletions pollers/select.c
Expand Up @@ -81,10 +81,10 @@ int io_select_add(io_select_poller *poller, io_atom *atom, int flags)
int fd = atom->fd;

if(fd < 0 || fd >= FD_SETSIZE) {
return -ERANGE;
return ERANGE;
}
if(poller->connections[fd]) {
return -EALREADY;
return EALREADY;
}

poller->connections[fd] = atom;
Expand All @@ -102,10 +102,10 @@ int io_select_set(io_select_poller *poller, io_atom *atom, int flags)
int fd = atom->fd;

if(fd < 0 || fd >= FD_SETSIZE) {
return -ERANGE;
return ERANGE;
}
if(!poller->connections[fd]) {
return -EEXIST;
return EEXIST;
}

install(poller, fd, flags);
Expand All @@ -119,10 +119,10 @@ int io_select_remove(io_select_poller *poller, io_atom *atom)
int fd = atom->fd;

if(fd < 0 || fd >= FD_SETSIZE) {
return -ERANGE;
return ERANGE;
}
if(!poller->connections[fd]) {
return -EALREADY;
return EALREADY;
}

install(poller, fd, 0);
Expand Down
15 changes: 4 additions & 11 deletions socket.c
Expand Up @@ -164,35 +164,28 @@ int io_socket_accept(io_poller *poller, io_atom *io, io_proc read_proc, io_proc
// see if we receive a connection.
continue;
}
if(errno == EAGAIN || errno == EWOULDBLOCK) {
// No pending connections are present (socket is nonblocking).
return -1;
}

// Probably there is a network error pending for this
// connection (already!). Should probably just ignore it...?
return -1;
return errno ? errno : -1;
}

// Excellent. We managed to connect to the remote socket.
if(set_nonblock(io->fd) < 0) {
close(io->fd);
return -1;
return errno ? errno : -1;
}

io_atom_init(io, io->fd, read_proc, write_proc);
err = io_add(poller, io, flags);
if(err < 0) {
close(io->fd);
return -1;
return err;
}

if(remote) {
remote->addr = pin.sin_addr;
remote->port = (int)ntohs(pin.sin_port);
}

return io->fd;
return 0;
}


Expand Down
5 changes: 2 additions & 3 deletions testmock.c
Expand Up @@ -317,9 +317,8 @@ void create_listener(io_poller *poller, const char *str)
exit(1);
}

// printf("Opened listening socket on %s:%d, fd=%d\n",
// /*inet_ntoa(sock.addr), sock.port,*/ "1",1, atom->fd );
printf("%d", atom->fd);
printf("Opened listening socket on %s:%d, fd=%d\n",
inet_ntoa(sock.addr), sock.port, atom->fd );
}


Expand Down

0 comments on commit cc8fe6b

Please sign in to comment.