Consistently save and restore errno

Some functions in dbus-transport-socket.c make a (wrapped)
socket syscall, then call other APIs, then test the result and
errno of the socket syscall.

This would break horribly if those "other APIs" overwrote errno with
their own value (... and this is part of why errno is an awful API).

Notably, if running under DBUS_VERBOSE, _dbus_verbose() is basically
fprintf(), which sets errno; and our Unix fd-passing support
makes calls of the form _dbus_verbose ("Read/wrote %i unix fds\n", n)
between the syscall and the result processing.

Maybe one day we'll convert all of dbus' syscall wrappers to either
raise a DBusError, or use the "negative errno" convention that systemd
borrowed from the Linux kernel, and in particular, we would need to
do that if we ever ported it to a platform where socket error reporting
was not basically errno. However, in practice everyone uses something
derived from BSD sockets, so "this sets errno, you know what errno is"
is a good enough internal API if we make sure to use it correctly.

Nothing calls _dbus_get_is_errno_nonzero(), so I just removed it instead
of converting it to the new calling convention.

Bug: https://bugs.freedesktop.org/show_bug.cgi?id=83625
This commit is contained in:
Simon McVittie 2014-09-08 20:18:22 +01:00
parent 7bcfcec523
commit c7fdbe77dd
7 changed files with 90 additions and 51 deletions

View file

@ -53,10 +53,14 @@ do_check_nonce (int fd, const DBusString *nonce, DBusError *error)
while (nleft)
{
int saved_errno;
n = _dbus_read_socket (fd, &p, nleft);
if (n == -1 && _dbus_get_is_errno_eintr())
saved_errno = _dbus_save_socket_errno ();
if (n == -1 && _dbus_get_is_errno_eintr (saved_errno))
;
else if (n == -1 && _dbus_get_is_errno_eagain_or_ewouldblock())
else if (n == -1 && _dbus_get_is_errno_eagain_or_ewouldblock (saved_errno))
_dbus_sleep_milliseconds (100);
else if (n==-1)
{

View file

@ -184,6 +184,7 @@ socket_handle_watch (DBusWatch *watch,
{
int client_fd;
int listen_fd;
int saved_errno;
listen_fd = dbus_watch_get_socket (watch);
@ -192,15 +193,17 @@ socket_handle_watch (DBusWatch *watch,
else
client_fd = _dbus_accept (listen_fd);
saved_errno = _dbus_save_socket_errno ();
if (client_fd < 0)
{
/* EINTR handled for us */
if (_dbus_get_is_errno_eagain_or_ewouldblock ())
if (_dbus_get_is_errno_eagain_or_ewouldblock (saved_errno))
_dbus_verbose ("No client available to accept after all\n");
else
_dbus_verbose ("Failed to accept a client connection: %s\n",
_dbus_strerror_from_errno ());
_dbus_strerror (saved_errno));
SERVER_UNLOCK (server);
}

View file

@ -3937,12 +3937,12 @@ _dbus_daemon_unpublish_session_bus_address (void)
* See if errno is EAGAIN or EWOULDBLOCK (this has to be done differently
* for Winsock so is abstracted)
*
* @returns #TRUE if errno == EAGAIN or errno == EWOULDBLOCK
* @returns #TRUE if e == EAGAIN or e == EWOULDBLOCK
*/
dbus_bool_t
_dbus_get_is_errno_eagain_or_ewouldblock (void)
_dbus_get_is_errno_eagain_or_ewouldblock (int e)
{
return errno == EAGAIN || errno == EWOULDBLOCK;
return e == EAGAIN || e == EWOULDBLOCK;
}
/**
@ -4199,4 +4199,16 @@ _dbus_append_address_from_socket (int fd,
return FALSE;
}
int
_dbus_save_socket_errno (void)
{
return errno;
}
void
_dbus_restore_socket_errno (int saved_errno)
{
errno = saved_errno;
}
/* tests in dbus-sysdeps-util.c */

View file

@ -3253,12 +3253,12 @@ _dbus_flush_caches (void)
* See if errno is EAGAIN or EWOULDBLOCK (this has to be done differently
* for Winsock so is abstracted)
*
* @returns #TRUE if errno == EAGAIN or errno == EWOULDBLOCK
* @returns #TRUE if e == EAGAIN or e == EWOULDBLOCK
*/
dbus_bool_t
_dbus_get_is_errno_eagain_or_ewouldblock (void)
_dbus_get_is_errno_eagain_or_ewouldblock (int e)
{
return errno == WSAEWOULDBLOCK;
return e == WSAEWOULDBLOCK;
}
/**
@ -3725,6 +3725,18 @@ _dbus_check_setuid (void)
return FALSE;
}
int
_dbus_save_socket_errno (void)
{
return errno;
}
void
_dbus_restore_socket_errno (int saved_errno)
{
_dbus_win_set_errno (saved_errno);
}
/** @} end of sysdeps-win */
/* tests in dbus-sysdeps-util.c */

View file

@ -721,34 +721,24 @@ _dbus_set_errno_to_zero (void)
#endif
}
/**
* See if errno is set
* @returns #TRUE if errno is not 0
*/
dbus_bool_t
_dbus_get_is_errno_nonzero (void)
{
return errno != 0;
}
/**
* See if errno is ENOMEM
* @returns #TRUE if errno == ENOMEM
* @returns #TRUE if e == ENOMEM
*/
dbus_bool_t
_dbus_get_is_errno_enomem (void)
_dbus_get_is_errno_enomem (int e)
{
return errno == ENOMEM;
return e == ENOMEM;
}
/**
* See if errno is EINTR
* @returns #TRUE if errno == EINTR
* @returns #TRUE if e == EINTR
*/
dbus_bool_t
_dbus_get_is_errno_eintr (void)
_dbus_get_is_errno_eintr (int e)
{
return errno == EINTR;
return e == EINTR;
}
/**
@ -756,9 +746,9 @@ _dbus_get_is_errno_eintr (void)
* @returns #TRUE if errno == EPIPE
*/
dbus_bool_t
_dbus_get_is_errno_epipe (void)
_dbus_get_is_errno_epipe (int e)
{
return errno == EPIPE;
return e == EPIPE;
}
/**
@ -766,10 +756,10 @@ _dbus_get_is_errno_epipe (void)
* @returns #TRUE if errno == ETOOMANYREFS
*/
dbus_bool_t
_dbus_get_is_errno_etoomanyrefs (void)
_dbus_get_is_errno_etoomanyrefs (int e)
{
#ifdef ETOOMANYREFS
return errno == ETOOMANYREFS;
return e == ETOOMANYREFS;
#else
return FALSE;
#endif

View file

@ -376,13 +376,14 @@ dbus_bool_t _dbus_generate_random_ascii (DBusString *str,
const char* _dbus_error_from_errno (int error_number);
const char* _dbus_error_from_system_errno (void);
int _dbus_save_socket_errno (void);
void _dbus_restore_socket_errno (int saved_errno);
void _dbus_set_errno_to_zero (void);
dbus_bool_t _dbus_get_is_errno_nonzero (void);
dbus_bool_t _dbus_get_is_errno_eagain_or_ewouldblock (void);
dbus_bool_t _dbus_get_is_errno_enomem (void);
dbus_bool_t _dbus_get_is_errno_eintr (void);
dbus_bool_t _dbus_get_is_errno_epipe (void);
dbus_bool_t _dbus_get_is_errno_etoomanyrefs (void);
dbus_bool_t _dbus_get_is_errno_eagain_or_ewouldblock (int e);
dbus_bool_t _dbus_get_is_errno_enomem (int e);
dbus_bool_t _dbus_get_is_errno_eintr (int e);
dbus_bool_t _dbus_get_is_errno_epipe (int e);
dbus_bool_t _dbus_get_is_errno_etoomanyrefs (int e);
const char* _dbus_strerror_from_errno (void);
void _dbus_disable_sigpipe (void);

View file

@ -247,13 +247,15 @@ read_data_into_auth (DBusTransport *transport,
DBusTransportSocket *socket_transport = (DBusTransportSocket*) transport;
DBusString *buffer;
int bytes_read;
int saved_errno;
*oom = FALSE;
_dbus_auth_get_buffer (transport->auth, &buffer);
bytes_read = _dbus_read_socket (socket_transport->fd,
buffer, socket_transport->max_bytes_read_per_iteration);
saved_errno = _dbus_save_socket_errno ();
_dbus_auth_return_buffer (transport->auth, buffer);
@ -267,16 +269,16 @@ read_data_into_auth (DBusTransport *transport,
{
/* EINTR already handled for us */
if (_dbus_get_is_errno_enomem ())
if (_dbus_get_is_errno_enomem (saved_errno))
{
*oom = TRUE;
}
else if (_dbus_get_is_errno_eagain_or_ewouldblock ())
else if (_dbus_get_is_errno_eagain_or_ewouldblock (saved_errno))
; /* do nothing, just return FALSE below */
else
{
_dbus_verbose ("Error reading from remote app: %s\n",
_dbus_strerror_from_errno ());
_dbus_strerror (saved_errno));
do_io_error (transport);
}
@ -299,6 +301,7 @@ write_data_from_auth (DBusTransport *transport)
{
DBusTransportSocket *socket_transport = (DBusTransportSocket*) transport;
int bytes_written;
int saved_errno;
const DBusString *buffer;
if (!_dbus_auth_get_bytes_to_send (transport->auth,
@ -308,6 +311,7 @@ write_data_from_auth (DBusTransport *transport)
bytes_written = _dbus_write_socket (socket_transport->fd,
buffer,
0, _dbus_string_get_length (buffer));
saved_errno = _dbus_save_socket_errno ();
if (bytes_written > 0)
{
@ -318,12 +322,12 @@ write_data_from_auth (DBusTransport *transport)
{
/* EINTR already handled for us */
if (_dbus_get_is_errno_eagain_or_ewouldblock ())
if (_dbus_get_is_errno_eagain_or_ewouldblock (saved_errno))
;
else
{
_dbus_verbose ("Error writing to remote app: %s\n",
_dbus_strerror_from_errno ());
_dbus_strerror (saved_errno));
do_io_error (transport);
}
}
@ -527,6 +531,7 @@ do_writing (DBusTransport *transport)
const DBusString *body;
int header_len, body_len;
int total_bytes_to_write;
int saved_errno;
if (total > socket_transport->max_bytes_written_per_iteration)
{
@ -584,6 +589,7 @@ do_writing (DBusTransport *transport)
&socket_transport->encoded_outgoing,
socket_transport->message_bytes_written,
total_bytes_to_write - socket_transport->message_bytes_written);
saved_errno = _dbus_save_socket_errno ();
}
else
{
@ -612,6 +618,7 @@ do_writing (DBusTransport *transport)
0, body_len,
unix_fds,
n);
saved_errno = _dbus_save_socket_errno ();
if (bytes_written > 0 && n > 0)
_dbus_verbose("Wrote %i unix fds\n", n);
@ -638,6 +645,8 @@ do_writing (DBusTransport *transport)
body_len -
(socket_transport->message_bytes_written - header_len));
}
saved_errno = _dbus_save_socket_errno ();
}
}
@ -651,7 +660,7 @@ do_writing (DBusTransport *transport)
* http://lists.freedesktop.org/archives/dbus/2008-March/009526.html
*/
if (_dbus_get_is_errno_eagain_or_ewouldblock () || _dbus_get_is_errno_epipe ())
if (_dbus_get_is_errno_eagain_or_ewouldblock (saved_errno) || _dbus_get_is_errno_epipe (saved_errno))
goto out;
/* Since Linux commit 25888e (from 2.6.37-rc4, Nov 2010), sendmsg()
@ -663,7 +672,7 @@ do_writing (DBusTransport *transport)
* https://bugs.freedesktop.org/show_bug.cgi?id=80163
*/
else if (_dbus_get_is_errno_etoomanyrefs ())
else if (_dbus_get_is_errno_etoomanyrefs (saved_errno))
{
/* We only send fds in the first byte of the message.
* ETOOMANYREFS cannot happen after.
@ -686,7 +695,7 @@ do_writing (DBusTransport *transport)
else
{
_dbus_verbose ("Error writing to remote app: %s\n",
_dbus_strerror_from_errno ());
_dbus_strerror (saved_errno));
do_io_error (transport);
goto out;
}
@ -730,6 +739,7 @@ do_reading (DBusTransport *transport)
int bytes_read;
int total;
dbus_bool_t oom;
int saved_errno;
_dbus_verbose ("fd = %d\n",socket_transport->fd);
@ -774,6 +784,8 @@ do_reading (DBusTransport *transport)
&socket_transport->encoded_incoming,
socket_transport->max_bytes_read_per_iteration);
saved_errno = _dbus_save_socket_errno ();
_dbus_assert (_dbus_string_get_length (&socket_transport->encoded_incoming) ==
bytes_read);
@ -823,6 +835,7 @@ do_reading (DBusTransport *transport)
buffer,
socket_transport->max_bytes_read_per_iteration,
fds, &n_fds);
saved_errno = _dbus_save_socket_errno ();
if (bytes_read >= 0 && n_fds > 0)
_dbus_verbose("Read %i unix fds\n", n_fds);
@ -834,28 +847,29 @@ do_reading (DBusTransport *transport)
{
bytes_read = _dbus_read_socket (socket_transport->fd,
buffer, socket_transport->max_bytes_read_per_iteration);
saved_errno = _dbus_save_socket_errno ();
}
_dbus_message_loader_return_buffer (transport->loader,
buffer);
}
if (bytes_read < 0)
{
/* EINTR already handled for us */
if (_dbus_get_is_errno_enomem ())
if (_dbus_get_is_errno_enomem (saved_errno))
{
_dbus_verbose ("Out of memory in read()/do_reading()\n");
oom = TRUE;
goto out;
}
else if (_dbus_get_is_errno_eagain_or_ewouldblock ())
else if (_dbus_get_is_errno_eagain_or_ewouldblock (saved_errno))
goto out;
else
{
_dbus_verbose ("Error reading from remote app: %s\n",
_dbus_strerror_from_errno ());
_dbus_strerror (saved_errno));
do_io_error (transport);
goto out;
}
@ -1129,6 +1143,8 @@ socket_do_iteration (DBusTransport *transport,
if (poll_fd.events)
{
int saved_errno;
if (flags & DBUS_ITERATION_BLOCK)
poll_timeout = timeout_milliseconds;
else
@ -1147,8 +1163,9 @@ socket_do_iteration (DBusTransport *transport,
again:
poll_res = _dbus_poll (&poll_fd, 1, poll_timeout);
saved_errno = _dbus_save_socket_errno ();
if (poll_res < 0 && _dbus_get_is_errno_eintr ())
if (poll_res < 0 && _dbus_get_is_errno_eintr (saved_errno))
goto again;
if (flags & DBUS_ITERATION_BLOCK)
@ -1191,7 +1208,7 @@ socket_do_iteration (DBusTransport *transport,
else
{
_dbus_verbose ("Error from _dbus_poll(): %s\n",
_dbus_strerror_from_errno ());
_dbus_strerror (saved_errno));
}
}