From 163555c7ab56132ee27e3e7d9a26eb985682c1b5 Mon Sep 17 00:00:00 2001 From: Havoc Pennington Date: Fri, 17 Aug 2007 16:43:57 +0000 Subject: 2007-08-17 Havoc Pennington * tools/dbus-launch-x11.c (set_address_in_x11): fix from Michael Lorenz to use long not int with XChangeProperty format 32 * dbus/dbus-sysdeps-util-unix.c (_dbus_write_pid_to_file_and_pipe): factor this out, and use the same code in _dbus_become_daemon (where the parent writes the pid file and to the pid pipe) and in bus_context_new (where the daemon writes its own pid file and to its own pid pipe) * bus/bus.c (bus_context_new): close the pid pipe after we print to it. Also, don't write the pid to the pipe twice when we fork, someone reported this bug a long time ago. --- ChangeLog | 15 +++++ bus/bus.c | 116 +++++++++++++--------------------- bus/main.c | 4 ++ dbus/dbus-sysdeps-util-unix.c | 144 ++++++++++++++++++++++++++---------------- dbus/dbus-sysdeps.h | 9 ++- tools/dbus-launch-x11.c | 7 +- 6 files changed, 160 insertions(+), 135 deletions(-) diff --git a/ChangeLog b/ChangeLog index 37ab97f9..0b44bc1b 100644 --- a/ChangeLog +++ b/ChangeLog @@ -1,3 +1,18 @@ +2007-08-17 Havoc Pennington + + * tools/dbus-launch-x11.c (set_address_in_x11): fix from Michael + Lorenz to use long not int with XChangeProperty format 32 + + * dbus/dbus-sysdeps-util-unix.c + (_dbus_write_pid_to_file_and_pipe): factor this out, and use the + same code in _dbus_become_daemon (where the parent writes the pid + file and to the pid pipe) and in bus_context_new (where the daemon + writes its own pid file and to its own pid pipe) + + * bus/bus.c (bus_context_new): close the pid pipe after we print + to it. Also, don't write the pid to the pipe twice when we fork, + someone reported this bug a long time ago. + 2007-08-03 Havoc Pennington * configure.in: add major/minor/micro version number AC_SUBST diff --git a/bus/bus.c b/bus/bus.c index 21e8037e..c2ea1157 100644 --- a/bus/bus.c +++ b/bus/bus.c @@ -665,7 +665,7 @@ bus_context_new (const DBusString *config_file, if (!_dbus_pipe_is_stdout_or_stderr (print_addr_pipe)) _dbus_pipe_close (print_addr_pipe, NULL); - + _dbus_string_free (&addr); } @@ -695,78 +695,48 @@ bus_context_new (const DBusString *config_file, } } - /* Now become a daemon if appropriate */ - if ((force_fork != FORK_NEVER && context->fork) || force_fork == FORK_ALWAYS) - { - DBusString u; - - if (context->pidfile) - _dbus_string_init_const (&u, context->pidfile); - - if (!_dbus_become_daemon (context->pidfile ? &u : NULL, - print_pid_pipe, - error)) - { - _DBUS_ASSERT_ERROR_IS_SET (error); - goto failed; - } - } - else - { - /* Need to write PID file for ourselves, not for the child process */ - if (context->pidfile != NULL) - { - DBusString u; - - _dbus_string_init_const (&u, context->pidfile); - - if (!_dbus_write_pid_file (&u, _dbus_getpid (), error)) - { - _DBUS_ASSERT_ERROR_IS_SET (error); - goto failed; - } - } - } - - /* Write PID if requested */ - if (print_pid_pipe != NULL && _dbus_pipe_is_valid (print_pid_pipe)) - { - DBusString pid; - int bytes; - - if (!_dbus_string_init (&pid)) - { - BUS_SET_OOM (error); - goto failed; - } - - if (!_dbus_string_append_int (&pid, _dbus_getpid ()) || - !_dbus_string_append (&pid, "\n")) - { - _dbus_string_free (&pid); - BUS_SET_OOM (error); - goto failed; - } - - bytes = _dbus_string_get_length (&pid); - if (_dbus_pipe_write (print_pid_pipe, &pid, 0, bytes, error) != bytes) - { - /* 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_stdout_or_stderr (print_pid_pipe)) - _dbus_pipe_close (print_pid_pipe, NULL); - - _dbus_string_free (&pid); - } - + /* Now become a daemon if appropriate and write out pid file in any case */ + { + DBusString u; + + if (context->pidfile) + _dbus_string_init_const (&u, context->pidfile); + + if ((force_fork != FORK_NEVER && context->fork) || force_fork == FORK_ALWAYS) + { + _dbus_verbose ("Forking and becoming daemon\n"); + + if (!_dbus_become_daemon (context->pidfile ? &u : NULL, + print_pid_pipe, + error)) + { + _DBUS_ASSERT_ERROR_IS_SET (error); + goto failed; + } + } + else + { + _dbus_verbose ("Fork not requested\n"); + + /* Need to write PID file and to PID pipe for ourselves, + * not for the child process. This is a no-op if the pidfile + * is NULL and print_pid_pipe is NULL. + */ + if (!_dbus_write_pid_to_file_and_pipe (context->pidfile ? &u : NULL, + print_pid_pipe, + _dbus_getpid (), + error)) + { + _DBUS_ASSERT_ERROR_IS_SET (error); + goto failed; + } + } + } + + if (print_pid_pipe && _dbus_pipe_is_valid (print_pid_pipe) && + !_dbus_pipe_is_stdout_or_stderr (print_pid_pipe)) + _dbus_pipe_close (print_pid_pipe, NULL); + if (!process_config_postinit (context, parser, error)) { _DBUS_ASSERT_ERROR_IS_SET (error); diff --git a/bus/main.c b/bus/main.c index 3652935e..161de19c 100644 --- a/bus/main.c +++ b/bus/main.c @@ -452,6 +452,10 @@ main (int argc, char **argv) exit (1); } + /* bus_context_new() closes the print_addr_pipe and + * print_pid_pipe + */ + setup_reload_pipe (bus_context_get_loop (context)); _dbus_set_signal_handler (SIGHUP, signal_handler); diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c index 38b7b756..df967a38 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -62,6 +62,7 @@ * @{ */ + /** * Does the chdir, fork, setsid, etc. to become a daemon process. * @@ -123,68 +124,27 @@ _dbus_become_daemon (const DBusString *pidfile, /* Get a predictable umask */ _dbus_verbose ("setting umask\n"); umask (022); + + _dbus_verbose ("calling setsid()\n"); + if (setsid () == -1) + _dbus_assert_not_reached ("setsid() failed"); + break; default: - if (pidfile) + if (!_dbus_write_pid_to_file_and_pipe (pidfile, print_pid_pipe, + child_pid, error)) { - _dbus_verbose ("parent writing pid file\n"); - if (!_dbus_write_pid_file (pidfile, - child_pid, - error)) - { - _dbus_verbose ("pid file write failed, killing child\n"); - kill (child_pid, SIGTERM); - return FALSE; - } + _dbus_verbose ("pid file or pipe write failed: %s\n", + error->message); + kill (child_pid, SIGTERM); + return FALSE; } - /* Write PID if requested */ - if (print_pid_pipe != NULL && _dbus_pipe_is_valid (print_pid_pipe)) - { - DBusString pid; - int bytes; - - if (!_dbus_string_init (&pid)) - { - _DBUS_SET_OOM (error); - kill (child_pid, SIGTERM); - return FALSE; - } - - if (!_dbus_string_append_int (&pid, child_pid) || - !_dbus_string_append (&pid, "\n")) - { - _dbus_string_free (&pid); - _DBUS_SET_OOM (error); - kill (child_pid, SIGTERM); - return FALSE; - } - - bytes = _dbus_string_get_length (&pid); - if (_dbus_pipe_write (print_pid_pipe, &pid, 0, bytes, error) != bytes) - { - /* _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; - } - - _dbus_string_free (&pid); - } _dbus_verbose ("parent exiting\n"); _exit (0); break; } - - _dbus_verbose ("calling setsid()\n"); - if (setsid () == -1) - _dbus_assert_not_reached ("setsid() failed"); return TRUE; } @@ -198,7 +158,7 @@ _dbus_become_daemon (const DBusString *pidfile, * @param error return location for errors * @returns #FALSE on failure */ -dbus_bool_t +static dbus_bool_t _dbus_write_pid_file (const DBusString *filename, unsigned long pid, DBusError *error) @@ -248,6 +208,84 @@ _dbus_write_pid_file (const DBusString *filename, return TRUE; } +/** + * Writes the given pid_to_write to a pidfile (if non-NULL) and/or to a + * pipe (if non-NULL). Does nothing if pidfile and print_pid_pipe are both + * NULL. + * + * @param pidfile the file to write to or #NULL + * @param print_pid_pipe the pipe to write to or #NULL + * @param pid_to_write the pid to write out + * @param error error on failure + * @returns FALSE if error is set + */ +dbus_bool_t +_dbus_write_pid_to_file_and_pipe (const DBusString *pidfile, + DBusPipe *print_pid_pipe, + dbus_pid_t pid_to_write, + DBusError *error) +{ + if (pidfile) + { + _dbus_verbose ("writing pid file %s\n", _dbus_string_get_const_data (pidfile)); + if (!_dbus_write_pid_file (pidfile, + pid_to_write, + error)) + { + _dbus_verbose ("pid file write failed\n"); + _DBUS_ASSERT_ERROR_IS_SET(error); + return FALSE; + } + } + else + { + _dbus_verbose ("No pid file requested\n"); + } + + if (print_pid_pipe != NULL && _dbus_pipe_is_valid (print_pid_pipe)) + { + DBusString pid; + int bytes; + + _dbus_verbose ("writing our pid to pipe %d\n", print_pid_pipe->fd_or_handle); + + if (!_dbus_string_init (&pid)) + { + _DBUS_SET_OOM (error); + return FALSE; + } + + if (!_dbus_string_append_int (&pid, pid_to_write) || + !_dbus_string_append (&pid, "\n")) + { + _dbus_string_free (&pid); + _DBUS_SET_OOM (error); + return FALSE; + } + + bytes = _dbus_string_get_length (&pid); + if (_dbus_pipe_write (print_pid_pipe, &pid, 0, bytes, error) != bytes) + { + /* _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); + return FALSE; + } + + _dbus_string_free (&pid); + } + else + { + _dbus_verbose ("No pid pipe to write to\n"); + } + + return TRUE; +} + /** * Verify that after the fork we can successfully change to this user. * diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index ab1631b8..eadfb433 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -363,13 +363,16 @@ void _dbus_print_backtrace (void); dbus_bool_t _dbus_become_daemon (const DBusString *pidfile, DBusPipe *print_pid_pipe, DBusError *error); -dbus_bool_t _dbus_write_pid_file (const DBusString *filename, - unsigned long pid, - DBusError *error); + dbus_bool_t _dbus_verify_daemon_user (const char *user); dbus_bool_t _dbus_change_to_daemon_user (const char *user, DBusError *error); +dbus_bool_t _dbus_write_pid_to_file_and_pipe (const DBusString *pidfile, + DBusPipe *print_pid_pipe, + dbus_pid_t pid_to_write, + DBusError *error); + /** A UNIX signal handler */ typedef void (* DBusSignalHandler) (int sig); diff --git a/tools/dbus-launch-x11.c b/tools/dbus-launch-x11.c index dfb28445..927d8634 100644 --- a/tools/dbus-launch-x11.c +++ b/tools/dbus-launch-x11.c @@ -342,7 +342,7 @@ set_address_in_x11(char *address, pid_t pid) { char *current_address; Window wid; - int pid32; + unsigned long pid32; /* Xlib property functions want _long_ not 32-bit for format "32" */ /* lock the X11 display to make sure we're doing this atomically */ XGrabServer (xdisplay); @@ -372,11 +372,6 @@ set_address_in_x11(char *address, pid_t pid) XChangeProperty (xdisplay, wid, address_atom, XA_STRING, 8, PropModeReplace, (unsigned char *)address, strlen (address)); pid32 = pid; - if (sizeof(pid32) != 4) - { - fprintf (stderr, "int is not 32 bits!\n"); - exit (1); - } XChangeProperty (xdisplay, wid, pid_atom, XA_CARDINAL, 32, PropModeReplace, (unsigned char *)&pid32, 1); -- cgit