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();
}
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();
}
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
