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

7 files changed, 90 insertions, 51 deletions

diff --git a/dbus/dbus-nonce.c b/dbus/dbus-nonce.c
index 37f30f00..44c46b2f 100644
--- a/dbus/dbus-nonce.c
+++ b/dbus/dbus-nonce.c
@@ -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)
         {
diff --git a/dbus/dbus-server-socket.c b/dbus/dbus-server-socket.c
index 060a919e..70367c79 100644
--- a/dbus/dbus-server-socket.c
+++ b/dbus/dbus-server-socket.c
@@ -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);
         }
diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index 544429a7..4985d3b7 100644
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -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 */
diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c
index 341db8ab..2c18f40e 100644
--- a/dbus/dbus-sysdeps-win.c
+++ b/dbus/dbus-sysdeps-win.c
@@ -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 */
 
diff --git a/dbus/dbus-sysdeps.c b/dbus/dbus-sysdeps.c
index f4ba0fac..99792100 100644
--- a/dbus/dbus-sysdeps.c
+++ b/dbus/dbus-sysdeps.c
@@ -722,33 +722,23 @@ _dbus_set_errno_to_zero (void)
 }
 
 /**
- * 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
diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
index 955ba9aa..03248f07 100644
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -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);
diff --git a/dbus/dbus-transport-socket.c b/dbus/dbus-transport-socket.c
index 199d3b54..c1e47018 100644
--- a/dbus/dbus-transport-socket.c
+++ b/dbus/dbus-transport-socket.c
@@ -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));
         }
     }