[Lldb-commits] [PATCH] D13772: Fix temporary directory computation on linux (pr25147)

[Pavel Labath via lldb-commits]

labath created this revision.
labath added reviewers: clayborg, tberghammer.
labath added subscribers: emaste, lldb-commits.
Herald added subscribers: srhines, danalbert, tberghammer.

On linux, the environment variables for temp directories that lldb checks for are generally not
defined, and the temp directory computation failed. This caused expression evaluation to fall
back to creating "/tmp/lldb-*.expr" debugging files instead of the usual
"$TMP/lldb/pid/lldb-*.expr". Crucially, these files were not cleaned up on lldb exit, which
caused clutter in the /tmp folder, especially on long-running machines (e.g. builtbots). This
commit fixes lldb to use llvm::sys::path::system_temp_directory, which does the same environment
variable dance, but (!) also falls back to the P_tmpdir macro, which is how the temp directory is
defined on linux.

Since the linux temp path computation now succeeds, I needed to also modify Android path
computation to check for actual directory existence, rather then checking whether the operation
failed.

http://reviews.llvm.org/D13772

Files:
  source/Host/android/HostInfoAndroid.cpp
  source/Host/common/HostInfoBase.cpp

Index: source/Host/common/HostInfoBase.cpp
===================================================================
--- source/Host/common/HostInfoBase.cpp
+++ source/Host/common/HostInfoBase.cpp
@@ -20,6 +20,7 @@
 #include "llvm/ADT/Triple.h"
 #include "llvm/ADT/StringExtras.h"
 #include "llvm/Support/Host.h"
+#include "llvm/Support/Path.h"
 #include "llvm/Support/raw_ostream.h"
 
 #include <thread>
@@ -344,19 +345,9 @@
 bool
 HostInfoBase::ComputeTempFileBaseDirectory(FileSpec &file_spec)
 {
-    file_spec.Clear();
-
-    const char *tmpdir_cstr = getenv("TMPDIR");
-    if (tmpdir_cstr == nullptr)
-    {
-        tmpdir_cstr = getenv("TMP");
-        if (tmpdir_cstr == nullptr)
-            tmpdir_cstr = getenv("TEMP");
-    }
-    if (!tmpdir_cstr)
-        return false;
-
-    file_spec = FileSpec(tmpdir_cstr, false);
+    llvm::SmallVector<char, 16> tmpdir;
+    llvm::sys::path::system_temp_directory(/*ErasedOnReboot*/ true, tmpdir);
+    file_spec = FileSpec(std::string(tmpdir.data(), tmpdir.size()), true);
     return true;
 }
 
Index: source/Host/android/HostInfoAndroid.cpp
===================================================================
--- source/Host/android/HostInfoAndroid.cpp
+++ source/Host/android/HostInfoAndroid.cpp
@@ -91,11 +91,14 @@
 bool
 HostInfoAndroid::ComputeTempFileBaseDirectory(FileSpec &file_spec)
 {
-    if (HostInfoLinux::ComputeTempFileBaseDirectory(file_spec))
-        return true;
+    bool success = HostInfoLinux::ComputeTempFileBaseDirectory(file_spec);
 
-    // If the default mechanism for computing the temp directory failed then
-    // fall back to /data/local/tmp
-    file_spec = FileSpec("/data/local/tmp", false);
-    return true;
+    // On Android, there is no path which is guaranteed to be writable. If the user has not
+    // provided a path via an environment variable, the generic algorithm will deduce /tmp, which
+    // is plain wrong. In that case we have an invalid directory, we substitute the path with
+    // /data/local/tmp, which is correct at least in some cases (i.e., when running as shell user).
+    if (!success || !file_spec.Exists())
+        file_spec = FileSpec("/data/local/tmp", false);
+
+    return file_spec.Exists();
 }


-------------- next part --------------
A non-text attachment was scrubbed...
Name: D13772.37485.patch
Type: text/x-patch
Size: 2222 bytes
Desc: not available
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20151015/9f6725f2/attachment.bin>