diff options
| -rw-r--r-- | ChangeLog | 14 | ||||
| -rw-r--r-- | bus/bus.c | 42 | ||||
| -rw-r--r-- | bus/bus.h | 4 | ||||
| -rw-r--r-- | bus/config-parser.c | 1 | ||||
| -rw-r--r-- | bus/main.c | 18 | ||||
| -rw-r--r-- | bus/test.c | 4 | ||||
| -rw-r--r-- | dbus/dbus-sysdeps-unix.c | 78 | ||||
| -rw-r--r-- | dbus/dbus-sysdeps-util-unix.c | 17 | ||||
| -rw-r--r-- | dbus/dbus-sysdeps.h | 28 | ||||
| -rw-r--r-- | tools/dbus-launch.c | 7 |
10 files changed, 140 insertions, 73 deletions
@@ -1,3 +1,17 @@ +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 + 2007-03-12 Ralf Habacker <ralf.habacker@freenet.de> * dbus/dbus-sysdeps-win.h, dbus/dbus-sysdeps-win.c, @@ -527,8 +527,8 @@ process_config_postinit (BusContext *context, BusContext* bus_context_new (const DBusString *config_file, ForceForkSetting force_fork, - DBusPipe print_addr_fd, - DBusPipe print_pid_fd, + DBusPipe *print_addr_pipe, + DBusPipe *print_pid_pipe, DBusError *error) { BusContext *context; @@ -598,12 +598,12 @@ bus_context_new (const DBusString *config_file, if (!dbus_server_allocate_data_slot (&server_data_slot)) _dbus_assert_not_reached ("second ref of server data slot failed"); - /* Note that we don't know whether the print_addr_fd is + /* Note that we don't know whether the print_addr_pipe is * one of the sockets we're using to listen on, or some * other random thing. But I think the answer is "don't do * that then" */ - if (_dbus_pipe_is_valid(print_addr_fd)) + if (print_addr_pipe != NULL && _dbus_pipe_is_valid (print_addr_pipe)) { DBusString addr; const char *a = bus_context_get_address (context); @@ -625,17 +625,20 @@ bus_context_new (const DBusString *config_file, } bytes = _dbus_string_get_length (&addr); - if (_dbus_pipe_write(print_addr_fd, &addr, 0, bytes) != bytes) + if (_dbus_pipe_write (print_addr_pipe, &addr, 0, bytes, error) != bytes) { - dbus_set_error (error, DBUS_ERROR_FAILED, - "Printing message bus address: %s\n", - _dbus_strerror (errno)); + /* pipe write returns an error on failure but not short write */ + if (error != NULL && !dbus_error_is_set (error)) + { + dbus_set_error (error, DBUS_ERROR_FAILED, + "Printing message bus address: did not write all bytes\n"); + } _dbus_string_free (&addr); goto failed; } - if (_dbus_pipe_is_special(print_addr_fd)) - _dbus_pipe_close(print_addr_fd, NULL); + if (!_dbus_pipe_is_stdout_or_stderr (print_addr_pipe)) + _dbus_pipe_close (print_addr_pipe, NULL); _dbus_string_free (&addr); } @@ -681,7 +684,7 @@ bus_context_new (const DBusString *config_file, _dbus_string_init_const (&u, context->pidfile); if (!_dbus_become_daemon (context->pidfile ? &u : NULL, - print_pid_fd, + print_pid_pipe, error)) { _DBUS_ASSERT_ERROR_IS_SET (error); @@ -706,7 +709,7 @@ bus_context_new (const DBusString *config_file, } /* 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; @@ -726,17 +729,20 @@ bus_context_new (const DBusString *config_file, } 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)); + /* pipe_write sets error on failure but 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); goto failed; } - if (_dbus_pipe_is_special (print_pid_fd)) - _dbus_pipe_close (print_pid_fd, NULL); + if (!_dbus_pipe_is_stdout_or_stderr (print_pid_pipe)) + _dbus_pipe_close (print_pid_pipe, NULL); _dbus_string_free (&pid); } @@ -70,8 +70,8 @@ typedef enum BusContext* bus_context_new (const DBusString *config_file, ForceForkSetting force_fork, - DBusPipe print_addr_fd, - DBusPipe print_pid_fd, + DBusPipe *print_addr_pipe, + DBusPipe *print_pid_pipe, DBusError *error); dbus_bool_t bus_context_reload_config (BusContext *context, DBusError *error); diff --git a/bus/config-parser.c b/bus/config-parser.c index db46893a..ea12ce7f 100644 --- a/bus/config-parser.c +++ b/bus/config-parser.c @@ -27,6 +27,7 @@ #include "selinux.h" #include <dbus/dbus-list.h> #include <dbus/dbus-internals.h> +#include <dbus/dbus-userdb.h> #include <string.h> typedef enum @@ -247,8 +247,8 @@ main (int argc, char **argv) DBusString addr_fd; DBusString pid_fd; const char *prev_arg; - DBusPipe print_addr_fd; - DBusPipe print_pid_fd; + DBusPipe print_addr_pipe; + DBusPipe print_pid_pipe; int i; dbus_bool_t print_address; dbus_bool_t print_pid; @@ -387,10 +387,10 @@ main (int argc, char **argv) usage (); } - print_addr_fd = _dbus_pipe_init(-1); + _dbus_pipe_invalidate (&print_addr_pipe); if (print_address) { - print_addr_fd = _dbus_pipe_init(1); /* stdout */ + _dbus_pipe_init_stdout (&print_addr_pipe); if (_dbus_string_get_length (&addr_fd) > 0) { long val; @@ -404,15 +404,15 @@ main (int argc, char **argv) exit (1); } - print_addr_fd = _dbus_pipe_init(val); + _dbus_pipe_init (&print_addr_pipe, val); } } _dbus_string_free (&addr_fd); - print_pid_fd = _dbus_pipe_init(-1); + _dbus_pipe_invalidate (&print_pid_pipe); if (print_pid) { - print_pid_fd = _dbus_pipe_init(1); /* stdout */ + _dbus_pipe_init_stdout (&print_pid_pipe); if (_dbus_string_get_length (&pid_fd) > 0) { long val; @@ -426,7 +426,7 @@ main (int argc, char **argv) exit (1); } - print_pid_fd = _dbus_pipe_init(val); + _dbus_pipe_init (&print_pid_pipe, val); } } _dbus_string_free (&pid_fd); @@ -439,7 +439,7 @@ main (int argc, char **argv) dbus_error_init (&error); context = bus_context_new (&config_file, force_fork, - print_addr_fd, print_pid_fd, + &print_addr_pipe, &print_pid_pipe, &error); _dbus_string_free (&config_file); if (context == NULL) @@ -298,7 +298,6 @@ bus_context_new_test (const DBusString *test_data_dir, DBusString config_file; DBusString relative; BusContext *context; - DBusPipe pipe; if (!_dbus_string_init (&config_file)) { @@ -324,8 +323,7 @@ bus_context_new_test (const DBusString *test_data_dir, } dbus_error_init (&error); - pipe = _dbus_pipe_init(-1); - context = bus_context_new (&config_file, FALSE, pipe, pipe, &error); + context = bus_context_new (&config_file, FALSE, NULL, NULL, &error); if (context == NULL) { _DBUS_ASSERT_ERROR_IS_SET (&error); 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, diff --git a/tools/dbus-launch.c b/tools/dbus-launch.c index d3072782..569a3ca6 100644 --- a/tools/dbus-launch.c +++ b/tools/dbus-launch.c @@ -596,14 +596,17 @@ babysit (int exit_with_session, exit (0); } -static void do_close_stderr (void) +static void +do_close_stderr (void) { + int fd; + fflush (stderr); /* dbus-launch is a Unix-only program, so we can rely on /dev/null being there. * We're including unistd.h and we're dealing with sh/csh launch sequences... */ - int fd = open ("/dev/null", O_RDWR); + fd = open ("/dev/null", O_RDWR); if (fd == -1) { fprintf (stderr, "Internal error: cannot open /dev/null: %s", strerror (errno)); |