[PATCH v2 03/11] sio: remove exceptions
Vladimir Davydov
vdavydov.dev at gmail.com
Sun Dec 9 15:54:13 MSK 2018
On Wed, Dec 05, 2018 at 12:28:50AM +0300, Vladislav Shpilevoy wrote:
> @@ -40,6 +40,14 @@
> #include "trivia/util.h"
> #include "exception.h"
>
> +enum { SIO_ERROR_FATAL = -2 };
> +
> +bool
> +sio_is_error_fatal(int retcode)
> +{
> + return retcode == SIO_ERROR_FATAL;
> +}
> +
> /** Pretty print socket name and peer (for exceptions) */
> const char *
> sio_socketname(int fd)
> @@ -224,9 +234,11 @@ sio_accept(int fd, struct sockaddr *addr, socklen_t *addrlen)
> {
> /* Accept a connection. */
> int newfd = accept(fd, addr, addrlen);
> - if (newfd < 0 &&
> - (errno != EAGAIN && errno != EWOULDBLOCK && errno != EINTR))
> - tnt_raise(SocketError, sio_socketname(fd), "accept");
> + if (newfd < 0 && errno != EAGAIN && errno != EWOULDBLOCK &&
> + errno != EINTR) {
> + diag_set(SocketError, sio_socketname(fd), "accept");
> + return SIO_ERROR_FATAL;
> + }
> return newfd;
> }
This looks bad. It's not just my opinion. I consulted with Kostja
verbally and he agrees with me.
Let's please set diag on any error in all sio functions and let the
caller decide which ones are critical and which are not by checking
errno. To simplify the caller's job we can introduce a helper function
checking errno for cetain types of errors, say
sio_wouldblock(errno): errno in EINTR, EAGAIN, EWOULDBLOCK
I've already suggested you to do this. Quoting your reply here:
> Any names having 'wouldblock', 'block' or describing other concrete
> behaviour can not be used. At least, sio_bind/listen can set EADDRINUSE,
> which is not about blocking.
Yeah, I agree, we should only use sio_wouldblock for EAGAIN, EINTR, and
EWOULDBLOCK. Other error codes should be checked explicitly.
>
> Also, notion of 'critical' is different for various functions. I've
> grouped errors in 4 groups:
>
> EINPROGRESS - sio_connect
Check (errno != EINPROGRESS) after calling sio_connect().
>
> EADDRINUSE - sio_bind, sio_listen
errno != EADDRINUSE
>
> EAGAIN - sio_accept, sio_write,
> EWOULDBLOCK sio_writev, sio_readn_ahead,
> EINTR sio_sendto, sio_recvfrom
sio_wouldblock(errno)
>
> ECONNRESET - sio_read
> EAGAIN
> EWOULDBLOCK
> EINTR
Actually, sio_read() never returns ECONNRESET - it treats it as EOF
(i.e. returns 0) and doesn't raise an error. So for sio_read(), the
caller can use sio_wouldblock() as well.
More information about the Tarantool-patches
mailing list