Consistently save and restore errno
authorSimon McVittie <simon.mcvittie@collabora.co.uk>
Mon, 8 Sep 2014 19:18:22 +0000 (20:18 +0100)
committerSimon McVittie <simon.mcvittie@collabora.co.uk>
Wed, 29 Oct 2014 14:02:37 +0000 (14:02 +0000)
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

dbus/dbus-nonce.c
dbus/dbus-server-socket.c
dbus/dbus-sysdeps-unix.c
dbus/dbus-sysdeps-win.c
dbus/dbus-sysdeps.c
dbus/dbus-sysdeps.h
dbus/dbus-transport-socket.c

index 37f30f0..44c46b2 100644 (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)
         {
index 060a919..70367c7 100644 (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);
         }
index 544429a..4985d3b 100644 (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 */
index 341db8a..2c18f40 100644 (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 */
 
index f4ba0fa..9979210 100644 (file)
@@ -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
index 955ba9a..03248f0 100644 (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);
index 199d3b5..c1e4701 100644 (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));
         }
     }