Re: [PATCH 3/3] Add DWARF index cache

[Simon Marchi <simon dot marchi at ericsson dot com>]

On 2018-07-17 03:48 PM, Tom Tromey wrote:
>>>>>> "Simon" == Simon Marchi <simon.marchi@ericsson.com> writes:
> 
> Simon> +/* The index cache directory, used for "set/show index-cache directory".  */
> Simon> +static char *index_cache_directory = NULL;
> 
> I think it would be good if the code that used this first passed it
> through tilde_expand, so users could "set index-cache directory ~/.blah".

Good point.  This is already done in do_set_command, because the parameter is of
type var_filename.  But it is not done in the case the path comes from XDG_CACHE_HOME,
for example.

Also, I think it would be good to store that path as an absolute path too.

Let's say I do:

(gdb) cd /tmp
(gdb) set index-cache directory foo
(gdb) cd /

I would expect the index-cache directory to stay /tmp/foo (relative to the current
working directory at the moment I entered the command).  Storing the directory
as a relative path would make it "follow" the current working directory changes,
which I think would be unintuitive.

So here's what I changed:

- Make get_standard_cache_dir return an absolute, tilde-expanded path.
- Make set_index_cache_directory_command transform index_cache_directory so it is
  absolute and tilde-expanded.


Here's the addendum to the patch (I also included an unrelated change, remove an
unnecessary rvalue-reference):


>From f091b3930e784394e2edc35a27d6c07468776f31 Mon Sep 17 00:00:00 2001
From: Simon Marchi <simon.marchi@ericsson.com>
Date: Tue, 17 Jul 2018 16:33:18 -0400
Subject: [PATCH] fixup

---
 gdb/common/pathstuff.c  | 12 ++++++++++--
 gdb/common/pathstuff.h  |  2 +-
 gdb/dwarf-index-cache.c |  6 +++++-
 gdb/dwarf-index-cache.h |  2 +-
 4 files changed, 17 insertions(+), 5 deletions(-)

diff --git a/gdb/common/pathstuff.c b/gdb/common/pathstuff.c
index b64d43a..f449f1a 100644
--- a/gdb/common/pathstuff.c
+++ b/gdb/common/pathstuff.c
@@ -166,11 +166,19 @@ get_standard_cache_dir ()
 {
   char *xdg_cache_home = getenv ("XDG_CACHE_HOME");
   if (xdg_cache_home != NULL)
-    return string_printf ("%s/gdb", xdg_cache_home);
+    {
+      /* Make sure the path is absolute and tilde-expanded.  */
+      gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (xdg_cache_home));
+      return string_printf ("%s/gdb", abs.get ());
+    }

   char *home = getenv ("HOME");
   if (home != NULL)
-    return string_printf ("%s/.cache/gdb", home);
+    {
+      /* Make sure the path is absolute and tilde-expanded.  */
+      gdb::unique_xmalloc_ptr<char> abs (gdb_realpath (home));
+      return string_printf ("%s/.cache/gdb", abs.get ());
+    }

   return {};
 }
diff --git a/gdb/common/pathstuff.h b/gdb/common/pathstuff.h
index 264700f..d1aa6b3 100644
--- a/gdb/common/pathstuff.h
+++ b/gdb/common/pathstuff.h
@@ -54,7 +54,7 @@ extern bool contains_dir_separator (const char *path);

    On Linux, it follows the XDG Base Directory specification: use
    $XDG_CACHE_HOME/gdb if the XDG_CACHE_HOME environment variable is defined,
-   otherwise $HOME/.cache.
+   otherwise $HOME/.cache.  The return value is absolute and tilde-expanded.

    Return an empty string if neither XDG_CACHE_HOME or HOME are defined.  */

diff --git a/gdb/dwarf-index-cache.c b/gdb/dwarf-index-cache.c
index 049d091..d3d669a 100644
--- a/gdb/dwarf-index-cache.c
+++ b/gdb/dwarf-index-cache.c
@@ -124,7 +124,7 @@ index_cache_resource::~index_cache_resource () = default;
 /* See dwarf-index-cache.h.  */

 void
-index_cache::set_directory (std::string &&dir)
+index_cache::set_directory (std::string dir)
 {
   gdb_assert (!dir.empty ());

@@ -345,6 +345,10 @@ static void
 set_index_cache_directory_command (const char *arg, int from_tty,
 				   cmd_list_element *element)
 {
+  /* Make sure the index cache directory is absolute and tilde-expanded.  */
+  gdb::unique_xmalloc_ptr<char> abs (gdb_abspath (index_cache_directory));
+  xfree (index_cache_directory);
+  index_cache_directory = abs.release ();
   global_index_cache.set_directory (index_cache_directory);
 }

diff --git a/gdb/dwarf-index-cache.h b/gdb/dwarf-index-cache.h
index 7b305c3..b4fa8c4 100644
--- a/gdb/dwarf-index-cache.h
+++ b/gdb/dwarf-index-cache.h
@@ -38,7 +38,7 @@ class index_cache
 {
 public:
   /* Change the directory used to save/load index files.  */
-  void set_directory (std::string &&dir);
+  void set_directory (std::string dir);

   /* Return true if the usage of the cache is enabled.  */
   bool enabled () const
-- 
2.7.4