2007-03-11 Havoc Pennington <hp@redhat.com>

	* tools/dbus-launch.c (do_close_stderr): fix C89 problem and
	formatting problem

	* Mostly fix the DBusPipe mess.
	- put line break after function return types
	- put space before parens
	- do not pass structs around by value
	- don't use dbus_strerror after calling supposedly cross-platform
	api
	- don't name pipe variables "fd"
	- abstract special fd numbers like -1 and 1

3 files changed, 84 insertions, 39 deletions

diff --git a/dbus/dbus-sysdeps-unix.c b/dbus/dbus-sysdeps-unix.c
index edd4025d..3b265d78 100644
--- a/dbus/dbus-sysdeps-unix.c
+++ b/dbus/dbus-sysdeps-unix.c
@@ -172,14 +172,25 @@ _dbus_write_socket (int               fd,
 /**
  * init a pipe instance.
  *
+ * @param pipe the pipe
  * @param fd the file descriptor to init from 
- * @returns a DBusPipe instance
  */
-DBusPipe _dbus_pipe_init(int         fd)
+void
+_dbus_pipe_init (DBusPipe *pipe,
+                 int       fd)
+{
+  pipe->fd_or_handle = fd;
+}
+
+/**
+ * init a pipe with stdout
+ *
+ * @param pipe the pipe
+ */
+void
+_dbus_pipe_init_stdout (DBusPipe *pipe)
 {
-  DBusPipe pipe;
-  pipe.fd = fd;
-  return pipe;
+  _dbus_pipe_init (pipe, 1);
 }
 
 /**
@@ -189,15 +200,26 @@ DBusPipe _dbus_pipe_init(int         fd)
  * @param buffer the buffer to write data from
  * @param start the first byte in the buffer to write
  * @param len the number of bytes to try to write
+ * @param error error return
  * @returns the number of bytes written or -1 on error
  */
 int
-_dbus_pipe_write (DBusPipe         pipe,
+_dbus_pipe_write (DBusPipe         *pipe,
                   const DBusString *buffer,
                   int               start,
-                  int               len)
+                  int               len,
+                  DBusError        *error)
 {
-  return _dbus_write (pipe.fd, buffer, start, len);
+  int written;
+  
+  written = _dbus_write (pipe->fd_or_handle, buffer, start, len);
+  if (written < 0)
+    {
+      dbus_set_error (error, DBUS_ERROR_FAILED,
+                      "Writing to pipe: %s\n",
+                      _dbus_strerror (errno));
+    }
+  return written;
 }
 
 /**
@@ -208,36 +230,54 @@ _dbus_pipe_write (DBusPipe         pipe,
  * @returns #FALSE if error is set
  */
 int
-_dbus_pipe_close  (DBusPipe         pipe,
+_dbus_pipe_close  (DBusPipe         *pipe,
                    DBusError        *error)
 {
-  return _dbus_close (pipe.fd, error);
+  if (_dbus_close (pipe->fd_or_handle, error) < 0)
+    {
+      return -1;
+    }
+  else
+    {
+      _dbus_pipe_invalidate (pipe);
+      return 0;
+    }
 }
 
 /**
- * check if a pipe is valid, which means is constructed
- * by a valid file descriptor
+ * check if a pipe is valid; pipes can be set invalid, similar to
+ * a -1 file descriptor.
  *
  * @param pipe the pipe instance
  * @returns #FALSE if pipe is not valid
  */
-dbus_bool_t _dbus_pipe_is_valid(DBusPipe pipe)
+dbus_bool_t
+_dbus_pipe_is_valid(DBusPipe *pipe)
 {
-  return pipe.fd >= 0;
+  return pipe->fd_or_handle >= 0;
 }
 
 /**
- * check if a pipe is a special pipe, which means using 
- * a non default file descriptor (>2)
+ * Check if a pipe is stdout or stderr.
  *
  * @param pipe the pipe instance
- * @returns #FALSE if pipe is not a special pipe
+ * @returns #TRUE if pipe is one of the standard out/err channels
  */
-dbus_bool_t _dbus_pipe_is_special(DBusPipe pipe)
+dbus_bool_t
+_dbus_pipe_is_stdout_or_stderr (DBusPipe *pipe)
 {
-  return pipe.fd > 2;
+  return pipe->fd_or_handle == 1 || pipe->fd_or_handle == 2;
 }
 
+/**
+ * Initializes a pipe to an invalid value.
+ * @param pipe the pipe
+ */
+void
+_dbus_pipe_invalidate (DBusPipe *pipe)
+{
+  pipe->fd_or_handle = -1;
+}
 
 /**
  * Like _dbus_write_two() but only works on sockets and is thus
diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c
index 5da57db4..5ffc90d9 100644
--- a/dbus/dbus-sysdeps-util-unix.c
+++ b/dbus/dbus-sysdeps-util-unix.c
@@ -61,13 +61,13 @@
  * Does the chdir, fork, setsid, etc. to become a daemon process.
  *
  * @param pidfile #NULL, or pidfile to create
- * @param print_pid_fd file descriptor to print daemon's pid to, or -1 for none
+ * @param print_pid_pipe pipe to print daemon's pid to, or -1 for none
  * @param error return location for errors
  * @returns #FALSE on failure
  */
 dbus_bool_t
 _dbus_become_daemon (const DBusString *pidfile,
-                     DBusPipe         print_pid_fd,
+                     DBusPipe         *print_pid_pipe,
                      DBusError        *error)
 {
   const char *s;
@@ -135,7 +135,7 @@ _dbus_become_daemon (const DBusString *pidfile,
         }
 
       /* Write PID if requested */
-      if (_dbus_pipe_is_valid(print_pid_fd))
+      if (print_pid_pipe != NULL && _dbus_pipe_is_valid (print_pid_pipe))
 	{
 	  DBusString pid;
 	  int bytes;
@@ -157,11 +157,14 @@ _dbus_become_daemon (const DBusString *pidfile,
 	    }
 	  
 	  bytes = _dbus_string_get_length (&pid);
-	  if (_dbus_pipe_write (print_pid_fd, &pid, 0, bytes) != bytes)
+	  if (_dbus_pipe_write (print_pid_pipe, &pid, 0, bytes, error) != bytes)
 	    {
-	      dbus_set_error (error, DBUS_ERROR_FAILED,
-			      "Printing message bus PID: %s\n",
-			      _dbus_strerror (errno));
+              /* _dbus_pipe_write sets error only on failure, not short write */
+              if (error != NULL && !dbus_error_is_set(error))
+                {
+                  dbus_set_error (error, DBUS_ERROR_FAILED,
+                                  "Printing message bus PID: did not write enough bytes\n");
+                }
 	      _dbus_string_free (&pid);
               kill (child_pid, SIGTERM);
 	      return FALSE;
diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h
index 879a47bc..4bdf2cc4 100644
--- a/dbus/dbus-sysdeps.h
+++ b/dbus/dbus-sysdeps.h
@@ -303,21 +303,23 @@ dbus_bool_t _dbus_path_is_absolute    (const DBusString *filename);
 dbus_bool_t _dbus_get_standard_session_servicedirs (DBusList **dirs);
 
 typedef struct {
-	int fd; 
+  int fd_or_handle;
 } DBusPipe;
 
-DBusPipe _dbus_pipe_init(int         fd);
-
-int _dbus_pipe_write (DBusPipe          pipe,
-                      const DBusString *buffer,
-                      int               start,
-                      int               len);
-
-int _dbus_pipe_close  (DBusPipe          pipe,
-					   DBusError        *error);
+void        _dbus_pipe_init                (DBusPipe         *pipe,
+                                            int               fd);
+void        _dbus_pipe_init_stdout         (DBusPipe         *pipe);
+int         _dbus_pipe_write               (DBusPipe         *pipe,
+                                            const DBusString *buffer,
+                                            int               start,
+                                            int               len,
+                                            DBusError        *error);
+int         _dbus_pipe_close               (DBusPipe         *pipe,
+                                            DBusError        *error);
+dbus_bool_t _dbus_pipe_is_valid            (DBusPipe         *pipe);
+void        _dbus_pipe_invalidate          (DBusPipe         *pipe);
+dbus_bool_t _dbus_pipe_is_stdout_or_stderr (DBusPipe         *pipe);
 
-dbus_bool_t _dbus_pipe_is_valid(DBusPipe pipe);
-dbus_bool_t _dbus_pipe_is_special(DBusPipe pipe);
 
 /** Opaque type for reading a directory listing */
 typedef struct DBusDirIter DBusDirIter;
@@ -385,7 +387,7 @@ dbus_bool_t _dbus_full_duplex_pipe (int              *fd1,
 void        _dbus_print_backtrace  (void);
 
 dbus_bool_t _dbus_become_daemon   (const DBusString *pidfile,
-                                   DBusPipe         print_pid_fd,
+                                   DBusPipe         *print_pid_pipe,
                                    DBusError        *error);
 dbus_bool_t _dbus_write_pid_file  (const DBusString *filename,
                                    unsigned long     pid,