[lldb-dev] Should FileSpec::Resolve() look at PATH?

[Zachary Turner]

I was a bit confused at how we could call a function named RunShellCommand
with an option to not run it in a shell, but I went and looked over the
code, and was surprised to find out this is actually a thing.  Seems we
pass NULL for the shell sometimes, with comments saying "Don't run in
shell".  Maybe someday I'll try to change the name of this function to
something less confusing, but for now I'll implement it as you suggest.


On Wed, Oct 15, 2014 at 9:52 AM, Greg Clayton <gclayton at apple.com> wrote:

> I would change the "shell" argument to a boolean "run_in_default_shell". I
> don't see any call sites of RunShellCommand that actually specify an
> alternate shell (though I may have missed one). Then if
> "run_in_default_shell" is true, we call GetDefaultShell().
>
> Greg
>
> > On Oct 15, 2014, at 9:27 AM, Zachary Turner <zturner at google.com> wrote:
> >
> > It's a little more difficult than that, because we have
> >
> > #define LLDB_DEFAULT_SHELL "cmd.exe"
> >
> > So we'd need to move this into a runtime function and fix up all
> assumptions about how it might be available at compile time.  It doesn't
> seem like that would be too bad though.   The biggest offender would be
> that we currently have a function like this:
> >
> > void Foo(int x, int y, const char *shell = LLDB_DEFAULT_SHELL)
> >
> > But I suppose we could just add a two-argument overload that calls the 3
> argument version with GetDefaultShell(), to avoid having to fix up a ton of
> callsites.
> >
> > This might be a better solution than fixing it in RunShellCommand
> because it removes the possibility of someone forgetting to resolve the
> path in the future.
> >
> > Thoughts Greg?
> >
> > On Tue, Oct 14, 2014 at 7:11 PM, Scott Graham <scottmg at chromium.org>
> wrote:
> > getenv("COMSPEC") rather than "cmd.exe"? I've never seen that be
> non-absolute.
> >
> > On Tue, Oct 14, 2014 at 4:36 PM, Zachary Turner <zturner at google.com>
> wrote:
> > I have an issue on Windows when trying to run shell commands.  We
> specify the shell as "cmd.exe", create a FileSpec out of this, and call
> FileSpec::Resolve.  This ends up making an absolute path out of cmd.exe,
> but it does so by just sticking the working directory onto the front of it,
> which is obviously wrong.
> >
> > My question is: For FileSpecs that are only filenames, nothing else,
> should we attempt to locate a matching file in PATH, and when we find one
> use the resulting absolute path?
> >
> > _______________________________________________
> > lldb-dev mailing list
> > lldb-dev at cs.uiuc.edu
> > http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev
> >
> >
> >
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-dev/attachments/20141015/d0e33844/attachment.html>