From b07755728b37e2a534267fb84468e620751cef6c Mon Sep 17 00:00:00 2001 From: Simon McVittie Date: Fri, 18 Sep 2015 17:53:14 +0100 Subject: [PATCH 7/7] Use DBusString for all relocation and install-root code --- bus/activation.c | 17 ++++- bus/config-parser.c | 52 +++++++------- dbus/dbus-sysdeps-util-unix.c | 13 ---- dbus/dbus-sysdeps-util-win.c | 159 +++++++++++++++++++----------------------- dbus/dbus-sysdeps-win.c | 83 ++++++++++++++++------ dbus/dbus-sysdeps-win.h | 2 +- dbus/dbus-sysdeps.h | 10 --- test/manual-paths.c | 44 ++++++------ 8 files changed, 197 insertions(+), 183 deletions(-) diff --git a/bus/activation.c b/bus/activation.c index 679a40e..546b690 100644 --- a/bus/activation.c +++ b/bus/activation.c @@ -261,6 +261,7 @@ update_desktop_file_entry (BusActivation *activation, DBusString file_path; DBusError tmp_error; dbus_bool_t retval; + DBusString str; _DBUS_ASSERT_ERROR_IS_CLEAR (error); @@ -308,9 +309,18 @@ update_desktop_file_entry (BusActivation *activation, error)) goto out; - exec = _dbus_strdup (_dbus_replace_install_prefix (exec_tmp)); - dbus_free (exec_tmp); - exec_tmp = NULL; + if (!_dbus_string_init (&str)) + goto out; + + if (!_dbus_string_append (&str, exec_tmp) || + !_dbus_replace_install_prefix_str (&str) || + !_dbus_string_steal_data (&str, &exec)) + { + _dbus_string_free (&str); + goto out; + } + + _dbus_string_free (&str); /* user is not _required_ unless we are using system activation */ if (!bus_desktop_file_get_string (desktop_file, @@ -466,6 +476,7 @@ update_desktop_file_entry (BusActivation *activation, out: /* if these have been transferred into entry, the variables will be NULL */ + dbus_free (exec_tmp); dbus_free (name); dbus_free (exec); dbus_free (user); diff --git a/bus/config-parser.c b/bus/config-parser.c index 0137b0d..e4b45f0 100644 --- a/bus/config-parser.c +++ b/bus/config-parser.c @@ -3406,17 +3406,25 @@ test_default_session_servicedirs (void) DBusList *link; DBusString progs; int i; + dbus_bool_t ret = FALSE; #ifdef DBUS_WIN const char *common_progs; - char buffer[1024]; + DBusString install_root_based; - if (_dbus_get_install_root(buffer, sizeof(buffer))) + if (!_dbus_string_init (&install_root_based) || + !_dbus_get_install_root (&install_root_based)) + _dbus_assert_not_reached ("OOM getting install root"); + + if (_dbus_string_get_length (&install_root_based) > 0) { - strcat(buffer,DBUS_DATADIR); - strcat(buffer,"/dbus-1/services"); - test_session_service_dir_matches[0] = buffer; + if (!_dbus_string_append (&install_root_based, DBUS_DATADIR) || + !_dbus_string_append (&install_root_based, "/dbus-1/services")) + _dbus_assert_not_reached ("OOM appending to install root"); + + test_session_service_dir_matches[0] = _dbus_string_get_const_data (&install_root_based); } + #endif /* On Unix we don't actually use this variable, but it's easier to handle the @@ -3430,16 +3438,11 @@ test_default_session_servicedirs (void) if (common_progs) { if (!_dbus_string_append (&progs, common_progs)) - { - _dbus_string_free (&progs); - return FALSE; - } + goto out; if (!_dbus_string_append (&progs, "/dbus-1/services")) - { - _dbus_string_free (&progs); - return FALSE; - } + goto out; + test_session_service_dir_matches[1] = _dbus_string_get_const_data(&progs); } #endif @@ -3461,8 +3464,7 @@ test_default_session_servicedirs (void) printf ("error with default session service directories\n"); dbus_free (link->data); _dbus_list_free_link (link); - _dbus_string_free (&progs); - return FALSE; + goto out; } dbus_free (link->data); @@ -3489,8 +3491,7 @@ test_default_session_servicedirs (void) printf ("more directories parsed than in match set\n"); dbus_free (link->data); _dbus_list_free_link (link); - _dbus_string_free (&progs); - return FALSE; + goto out; } if (strcmp (test_session_service_dir_matches[i], @@ -3501,8 +3502,7 @@ test_default_session_servicedirs (void) test_session_service_dir_matches[i]); dbus_free (link->data); _dbus_list_free_link (link); - _dbus_string_free (&progs); - return FALSE; + goto out; } ++i; @@ -3515,13 +3515,17 @@ test_default_session_servicedirs (void) { printf ("extra data %s in the match set was not matched\n", test_session_service_dir_matches[i]); - - _dbus_string_free (&progs); - return FALSE; + goto out; } - + + ret = TRUE; + +out: _dbus_string_free (&progs); - return TRUE; +#ifdef DBUS_WIN + _dbus_string_free (&install_root_based); +#endif + return ret; } static const char *test_system_service_dir_matches[] = diff --git a/dbus/dbus-sysdeps-util-unix.c b/dbus/dbus-sysdeps-util-unix.c index 8e61e5b..668f20a 100644 --- a/dbus/dbus-sysdeps-util-unix.c +++ b/dbus/dbus-sysdeps-util-unix.c @@ -1272,19 +1272,6 @@ fail: return FALSE; } -/* - * replaces the term DBUS_PREFIX in configure_time_path by the - * current dbus installation directory. On unix this function is a noop - * - * @param configure_time_path - * @return real path - */ -const char * -_dbus_replace_install_prefix (const char *configure_time_path) -{ - return configure_time_path; -} - /** * Replace the DBUS_PREFIX in the given path, in-place, by the * current D-Bus installation directory. On Unix this function diff --git a/dbus/dbus-sysdeps-util-win.c b/dbus/dbus-sysdeps-util-win.c index 244c7d8..e782e39 100644 --- a/dbus/dbus-sysdeps-util-win.c +++ b/dbus/dbus-sysdeps-util-win.c @@ -1469,60 +1469,6 @@ _dbus_command_for_pid (unsigned long pid, return FALSE; } -/* - * replaces the term DBUS_PREFIX in configure_time_path by the - * current dbus installation directory. On unix this function is a noop - * - * @param configure_time_path - * @return real path - */ -const char * -_dbus_replace_install_prefix (const char *configure_time_path) -{ -#ifndef DBUS_PREFIX - return configure_time_path; -#else - static char retval[1000]; - static char runtime_prefix[1000]; - int len = 1000; - int i; - - if (!configure_time_path) - return NULL; - - if ((!_dbus_get_install_root(runtime_prefix, len) || - strncmp (configure_time_path, DBUS_PREFIX "/", - strlen (DBUS_PREFIX) + 1))) { - strncpy (retval, configure_time_path, sizeof (retval) - 1); - /* strncpy does not guarantee to 0-terminate the string */ - retval[sizeof (retval) - 1] = '\0'; - } else { - size_t remaining; - - strncpy (retval, runtime_prefix, sizeof (retval) - 1); - retval[sizeof (retval) - 1] = '\0'; - remaining = sizeof (retval) - 1 - strlen (retval); - strncat (retval, - configure_time_path + strlen (DBUS_PREFIX) + 1, - remaining); - } - - /* Somehow, in some situations, backslashes get collapsed in the string. - * Since windows C library accepts both forward and backslashes as - * path separators, convert all backslashes to forward slashes. - */ - - for(i = 0; retval[i] != '\0'; i++) { - if(retval[i] == '\\') - retval[i] = '/'; - } - return retval; -#endif -} - -/* FIXME: in the development branch, redo _dbus_replace_install_prefix - * and _dbus_get_install_root to be DBusString-based instead of using - * statically sized buffers */ /** * Replace the DBUS_PREFIX in the given path, in-place, by the * current D-Bus installation directory. On Unix this function @@ -1534,41 +1480,57 @@ _dbus_replace_install_prefix (const char *configure_time_path) dbus_bool_t _dbus_replace_install_prefix_str (DBusString *path) { - const char *replaced; - DBusString replaced_str; +#ifndef DBUS_PREFIX + /* leave path unchanged */ + return TRUE; +#else + DBusString runtime_prefix; + int i; - replaced = _dbus_replace_install_prefix (_dbus_string_get_data (path)); + if (!_dbus_string_init (&runtime_prefix)) + return FALSE; - /* fast-path: nothing to do */ - if (_dbus_string_equal_c_str (path, replaced)) - return TRUE; + if (!_dbus_get_install_root (&runtime_prefix)) + { + _dbus_string_free (&runtime_prefix); + return FALSE; + } - _dbus_string_init_const (&replaced_str, replaced); + if (_dbus_string_get_length (&runtime_prefix) == 0) + { + /* cannot determine install root, leave path unchanged */ + _dbus_string_free (&runtime_prefix); + return TRUE; + } - /* note unusual calling convention: source is first, then dest */ - if (!_dbus_string_replace_len (&replaced_str, 0, strlen (replaced), - path, 0, _dbus_string_get_length (path))) - return FALSE; + if (_dbus_string_starts_with_c_str (path, DBUS_PREFIX "/")) + { + /* Replace DBUS_PREFIX "/" with runtime_prefix. + * Note unusual calling convention: source is first, then dest */ + if (!_dbus_string_replace_len ( + &runtime_prefix, 0, _dbus_string_get_length (&runtime_prefix), + path, 0, strlen (DBUS_PREFIX) + 1)) + { + _dbus_string_free (&runtime_prefix); + return FALSE; + } + } - return TRUE; -} + /* Somehow, in some situations, backslashes get collapsed in the string. + * Since windows C library accepts both forward and backslashes as + * path separators, convert all backslashes to forward slashes. + */ -/** - * return the relocated DATADIR - * - * @returns relocated DATADIR static string - */ + for (i = 0; i < _dbus_string_get_length (path); i++) + { + if (_dbus_string_get_byte (path, i) == '\\') + _dbus_string_set_byte (path, i, '/'); + } -static const char * -_dbus_windows_get_datadir (void) -{ - return _dbus_replace_install_prefix(DBUS_DATADIR); + return TRUE; +#endif } -#undef DBUS_DATADIR -#define DBUS_DATADIR _dbus_windows_get_datadir () - - #define DBUS_STANDARD_SESSION_SERVICEDIR "/dbus-1/services" #define DBUS_STANDARD_SYSTEM_SERVICEDIR "/dbus-1/system-services" @@ -1616,23 +1578,39 @@ _dbus_get_standard_session_servicedirs (DBusList **dirs) the code for accessing services requires absolute base pathes in case DBUS_DATADIR is relative make it absolute */ -#ifdef DBUS_WIN { DBusString p; - _dbus_string_init_const (&p, DBUS_DATADIR); + if (!_dbus_string_init (&p)) + goto oom; + + if (!_dbus_string_append (&p, DBUS_DATADIR)) + { + _dbus_string_free (&p); + goto oom; + } if (!_dbus_path_is_absolute (&p)) { - char install_root[1000]; - if (_dbus_get_install_root (install_root, sizeof(install_root))) - if (!_dbus_string_append (&servicedir_path, install_root)) + /* this only works because this is the first thing in the + * servicedir_path; if it wasn't, we'd have to use a temporary + * string and copy it in */ + if (!_dbus_get_install_root (&servicedir_path)) + { + _dbus_string_free (&p); goto oom; + } + } + + if (!_dbus_string_append (&servicedir_path, + _dbus_string_get_const_data (&p))) + { + _dbus_string_free (&p); + goto oom; } + + _dbus_string_free (&p); } -#endif - if (!_dbus_string_append (&servicedir_path, DBUS_DATADIR)) - goto oom; if (!_dbus_string_append (&servicedir_path, _DBUS_PATH_SEPARATOR)) goto oom; @@ -1693,7 +1671,10 @@ _dbus_get_config_file_name (DBusString *str, { DBusString tmp; - if (!_dbus_string_append (str, _dbus_windows_get_datadir ())) + if (!_dbus_string_append (str, DBUS_DATADIR)) + return FALSE; + + if (!_dbus_replace_install_prefix_str (str)) return FALSE; _dbus_string_init_const (&tmp, "dbus-1"); diff --git a/dbus/dbus-sysdeps-win.c b/dbus/dbus-sysdeps-win.c index 69644b0..7ddb7fd 100644 --- a/dbus/dbus-sysdeps-win.c +++ b/dbus/dbus-sysdeps-win.c @@ -2812,15 +2812,12 @@ _dbus_get_install_root_as_hash(DBusString *out) { DBusString install_path; - char path[MAX_PATH*2]; - int path_size = sizeof(path); + _dbus_string_init(&install_path); - if (!_dbus_get_install_root(path,path_size)) + if (!_dbus_get_install_root (&install_path) || + _dbus_string_get_length (&install_path) == 0) return FALSE; - _dbus_string_init(&install_path); - _dbus_string_append(&install_path,path); - _dbus_string_init(out); _dbus_string_tolower_ascii(&install_path,0,_dbus_string_get_length(&install_path)); @@ -3288,34 +3285,73 @@ _dbus_get_is_errno_eagain_or_ewouldblock (int e) } /** - * return the absolute path of the dbus installation + * Fill str with the absolute path of the D-Bus installation, or truncate str + * to zero length if we cannot determine it. * - * @param prefix buffer for installation path - * @param len length of buffer - * @returns #FALSE on failure + * @param str buffer for installation path + * @returns #FALSE on OOM, #TRUE if not OOM */ dbus_bool_t -_dbus_get_install_root(char *prefix, int len) +_dbus_get_install_root (DBusString *str) { - //To find the prefix, we cut the filename and also \bin\ if present - DWORD pathLength; + /* this is just an initial guess */ + DWORD pathLength = MAX_PATH; char *lastSlash; - SetLastError( 0 ); - pathLength = GetModuleFileNameA(_dbus_win_get_dll_hmodule(), prefix, len); - if ( pathLength == 0 || GetLastError() != 0 ) { - *prefix = '\0'; - return FALSE; - } + char *prefix; + + do + { + /* allocate enough space for our best guess at the length */ + if (!_dbus_string_set_length (str, pathLength)) + { + _dbus_string_set_length (str, 0); + return FALSE; + } + + SetLastError (0); + pathLength = GetModuleFileNameA (_dbus_win_get_dll_hmodule (), + _dbus_string_get_data (str), _dbus_string_get_length (str)); + + if (pathLength == 0 || GetLastError () != 0) + { + /* failed, but not OOM */ + _dbus_string_set_length (str, 0); + return TRUE; + } + + /* if the return is strictly less than the buffer size, it has + * not been truncated, so we can continue */ + if (pathLength < (DWORD) _dbus_string_get_length (str)) + { + /* reduce the length to match what Windows filled in */ + if (!_dbus_string_set_length (str, pathLength)) + { + _dbus_string_set_length (str, 0); + return FALSE; + } + + break; + } + + /* else it may have been truncated; try with a larger buffer */ + pathLength *= 2; + } + while (TRUE); + + /* the rest of this function works by direct byte manipulation of the + * underlying buffer */ + prefix = _dbus_string_get_data (str); + lastSlash = _mbsrchr(prefix, '\\'); if (lastSlash == NULL) { - *prefix = '\0'; - return FALSE; + /* failed, but not OOM */ + _dbus_string_set_length (str, 0); + return TRUE; } //cut off binary name lastSlash[1] = 0; //cut possible "\\bin" - //this fails if we are in a double-byte system codepage and the //folder's name happens to end with the *bytes* //"\\bin"... (I.e. the second byte of some Han character and then @@ -3327,6 +3363,9 @@ _dbus_get_install_root(char *prefix, int len) else if (lastSlash - prefix >= 12 && strnicmp(lastSlash - 12, "\\bin\\release", 12) == 0) lastSlash[-11] = 0; + /* fix up the length to match the byte-manipulation */ + _dbus_string_set_length (str, strlen (prefix)); + return TRUE; } diff --git a/dbus/dbus-sysdeps-win.h b/dbus/dbus-sysdeps-win.h index e9b30d7..0b5d8f0 100644 --- a/dbus/dbus-sysdeps-win.h +++ b/dbus/dbus-sysdeps-win.h @@ -85,7 +85,7 @@ _dbus_win_sid_to_name_and_domain (dbus_uid_t uid, dbus_bool_t _dbus_file_exists (const char *filename); DBUS_PRIVATE_EXPORT -dbus_bool_t _dbus_get_install_root(char *prefix, int len); +dbus_bool_t _dbus_get_install_root (DBusString *str); void _dbus_threads_windows_init_global (void); void _dbus_threads_windows_ensure_ctor_linked (void); diff --git a/dbus/dbus-sysdeps.h b/dbus/dbus-sysdeps.h index 260417d..48246f1 100644 --- a/dbus/dbus-sysdeps.h +++ b/dbus/dbus-sysdeps.h @@ -647,16 +647,6 @@ dbus_bool_t _dbus_change_to_daemon_user (const char *user, DBUS_PRIVATE_EXPORT void _dbus_flush_caches (void); -/* - * replaces the term DBUS_PREFIX in configure_time_path by the - * current dbus installation directory. On unix this function is a noop - * - * @param configure_time_path - * @return real path - */ -const char * -_dbus_replace_install_prefix (const char *configure_time_path); - dbus_bool_t _dbus_replace_install_prefix_str (DBusString *path); /* Do not set this too high: it is a denial-of-service risk. diff --git a/test/manual-paths.c b/test/manual-paths.c index 5116c16..f571bff 100644 --- a/test/manual-paths.c +++ b/test/manual-paths.c @@ -14,14 +14,31 @@ static dbus_bool_t print_install_root() { - char runtime_prefix[1000]; + DBusString runtime_prefix; - if (!_dbus_get_install_root(runtime_prefix, sizeof(runtime_prefix))) + if (!_dbus_string_init (&runtime_prefix)) { - fprintf(stderr, "dbus_get_install_root() failed\n"); + _dbus_assert_not_reached ("out of memory"); return FALSE; } - fprintf(stdout, "dbus_get_install_root() returned '%s'\n", runtime_prefix); + + if (!_dbus_get_install_root (&runtime_prefix)) + { + _dbus_assert_not_reached ("out of memory"); + _dbus_string_free (&runtime_prefix); + return FALSE; + } + + if (_dbus_string_get_length (&runtime_prefix) == 0) + { + fprintf (stderr, "_dbus_get_install_root() failed\n"); + _dbus_string_free (&runtime_prefix); + return FALSE; + } + + fprintf (stdout, "_dbus_get_install_root() returned '%s'\n", + _dbus_string_get_const_data (&runtime_prefix)); + _dbus_string_free (&runtime_prefix); return TRUE; } @@ -47,9 +64,6 @@ static dbus_bool_t print_service_dirs() static dbus_bool_t print_replace_install_prefix(const char *s) { DBusString str; - const char *s2 = _dbus_replace_install_prefix(s); - if (!s2) - return FALSE; if (!_dbus_string_init (&str)) { @@ -65,21 +79,9 @@ static dbus_bool_t print_replace_install_prefix(const char *s) return FALSE; } - if (!_dbus_string_equal_c_str (&str, s2)) - { - printf ("old API: length %lu \"%s\"\n", - (unsigned long) strlen (s2), s2); - printf ("new API: length %lu \"%s\"\n", - (unsigned long) _dbus_string_get_length (&str), - _dbus_string_get_const_data (&str)); - _dbus_assert_not_reached ("install prefix replaced differently"); - _dbus_string_free (&str); - return FALSE; - } - + fprintf(stdout, "replaced '%s' by '%s'\n", s, + _dbus_string_get_const_data (&str)); _dbus_string_free (&str); - - fprintf(stdout, "replaced '%s' by '%s'\n", s, s2); return TRUE; } -- 2.5.1