<div dir="ltr">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.<div><br></div></div><div class="gmail_extra"><br><div class="gmail_quote">On Wed, Oct 15, 2014 at 9:52 AM, Greg Clayton <span dir="ltr"><<a href="mailto:gclayton@apple.com" target="_blank">gclayton@apple.com</a>></span> wrote:<br><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">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().<br>
<span class="HOEnZb"><font color="#888888"><br>
Greg<br>
</font></span><div class="HOEnZb"><div class="h5"><br>
> On Oct 15, 2014, at 9:27 AM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
><br>
> It's a little more difficult than that, because we have<br>
><br>
> #define LLDB_DEFAULT_SHELL "cmd.exe"<br>
><br>
> 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:<br>
><br>
> void Foo(int x, int y, const char *shell = LLDB_DEFAULT_SHELL)<br>
><br>
> 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.<br>
><br>
> 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.<br>
><br>
> Thoughts Greg?<br>
><br>
> On Tue, Oct 14, 2014 at 7:11 PM, Scott Graham <<a href="mailto:scottmg@chromium.org">scottmg@chromium.org</a>> wrote:<br>
> getenv("COMSPEC") rather than "cmd.exe"? I've never seen that be non-absolute.<br>
><br>
> On Tue, Oct 14, 2014 at 4:36 PM, Zachary Turner <<a href="mailto:zturner@google.com">zturner@google.com</a>> wrote:<br>
> 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.<br>
><br>
> 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?<br>
><br>
> _______________________________________________<br>
> lldb-dev mailing list<br>
> <a href="mailto:lldb-dev@cs.uiuc.edu">lldb-dev@cs.uiuc.edu</a><br>
> <a href="http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev" target="_blank">http://lists.cs.uiuc.edu/mailman/listinfo/lldb-dev</a><br>
><br>
><br>
><br>
<br>
</div></div></blockquote></div><br></div>