Attachment 118350 Details for Bug 83539 – [untested] Use DBusString for all relocation and install-root code

[untested] Use DBusString for all relocation and install-root code

0007-Use-DBusString-for-all-relocation-and-install-root-c.patch (text/plain), 19.08 KB, created by Simon McVittie on 2015-09-18 17:00:28 UTC

(hide)

Description:

Filename:

MIME Type:

Creator: Simon McVittie

Created: 2015-09-18 17:00:28 UTC

Size: 19.08 KB

patch

obsolete

>From b07755728b37e2a534267fb84468e620751cef6c Mon Sep 17 00:00:00 2001
>From: Simon McVittie <simon.mcvittie@collabora.co.uk>
>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
>

Actions: View

Attachments on bug 83539: 105816 | 106930 | 106931 | 113363 | 113364 | 113377 | 118346 | 118349 | 118350 | 118540 | 118617 | 118618 | 118652

Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.