[Lldb-commits] [PATCH] FileSpec::Resolve should not turn a filename-only FileSpec into a qualified FileSpec if that file doesn't exist

[Zachary Turner]

http://lists.cs.uiuc.edu/pipermail/lldb-dev/2014-October/005551.html

Here's a thread that provides some context for why this was changed.

Your failure mode mentions invoking "lldb ls". If FileSpec::Resolve just
ends up returning "ls" unchanged, who actually resolves it? In the thread i
linked above, it seems i was also dealing with the same issue. Perhaps it's
time to add a function to resolve an executable using PATH? We could put it
in llvm::sys::fs and call it llvm::sys::resolve_executable(StringRef Exe,
bool use_path). Seems generally useful outside of lldb. The LLDB could just
always call this function before constructing a FileSpec to a target
executable.

On Fri, Feb 6, 2015 at 5:35 PM Zachary Turner <zturner at google.com> wrote:

> Could we add a test that verifies the behavior of this function?  Greg
> created an SBFileSpec test recently in lldb/test/functionalities/paths/TestPaths.py.
> As long as we don't use a builtin command like ls or something like it
> should be fine.  Instead you could use something like sys.executable, then
> the os.path module to get out the filename, then verify that after you
> resolve it, it matches sys.executable.
>
>
> ================
> Comment at: source/Host/common/FileSpec.cpp:167-173
> @@ -166,1 +166,9 @@
>
> +    // Save a copy of the original path that's passed in
> +    char original_path[PATH_MAX];
> +    ::strncpy (original_path, path.data(), sizeof (original_path));
> +    if (path.size() < sizeof (original_path))
> +        original_path[path.size()] = '\0';
> +    else
> +        original_path[sizeof (original_path) - 1] = '\0';
> +
> ----------------
> You can write this like this:
>
> llvm::SmallString<PATH_MAX> original_path(path.begin(),  path.end());
>
> ================
> Comment at: source/Host/common/FileSpec.cpp:177-183
> @@ +176,9 @@
> +
> +    // Get the resolved path as a char*, call stat() to see if it exists
> +    char resolved_path[PATH_MAX];
> +    ::strncpy (resolved_path, path.data(), sizeof (resolved_path));
> +    if (path.size() < sizeof (original_path))
> +        resolved_path[path.size()] = '\0';
> +    else
> +        resolved_path[sizeof (resolved_path) - 1] = '\0';
> +
> ----------------
> I'm preeeety sure that make_absolute() guarantees the result will be null
> terminated.  But not quite 100%.  If you can determine that it's guaranteed
> to be null terminated, you can replace all of this with path.data(), which
> returns a char*.  If you want to be extra certain though, you can still
> make this shorter by writing this instead:
>
> path.push_back(0);
> path.pop_back();
> if (stat(path.data())) // path.data() is now definitely null terminated.
>
> ================
> Comment at: source/Host/common/FileSpec.cpp:190-191
> @@ +189,4 @@
> +        // Return the original path instead
> +        path.resize (strlen (original_path) + 1);
> +        memcpy (path.data(), original_path, strlen (original_path) + 1);
> +    }
> ----------------
> This can just be path = original_path;
>
> http://reviews.llvm.org/D7477
>
> EMAIL PREFERENCES
>   http://reviews.llvm.org/settings/panel/emailpreferences/
>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20150207/7ef919c3/attachment.html>