labath added inline comments.
================
Comment at: include/lldb/Utility/Args.h:121
+ bool GetFlattenWindowsCommandString(std::string &command) const;
+
----------------
Hui wrote:
> labath wrote:
> > Hui wrote:
> > > labath wrote:
> > > > labath wrote:
> > > > > Hui wrote:
> > > > > > labath wrote:
> > > > > > > I am sorry for being such a pain, but I don't think this is
> > > > > > > grammatically correct, as `get` and `flatten` are both verbs. I'd
> > > > > > > call this either GetFlatten**ed**WindowsCommandLine or just plain
> > > > > > > FlattenWindowsCommandLine.
> > > > > > There are two kinds of sources of args in this discussion:
> > > > > >
> > > > > > - from lldb console through stdin which is not raw.
> > > > > > - from LLDB_SERVER_LOG_FILE environment variables which are raw
> > > > > >
> > > > > > We expect to call a GetFlattenedWindowsCommandString for raw input.
> > > > > > However it seems not the case for now.
> > > > > >
> > > > > > What about adding a TODO in the following in
> > > > > > ProcessWindowsLauncher.
> > > > > >
> > > > > > Would like a solution to proceed.
> > > > > >
> > > > > > +
> > > > > > +bool GetFlattenedWindowsCommandString(Args args, std::string
> > > > > > &command) {
> > > > > > + if (args.empty())
> > > > > > + return false;
> > > > > > +
> > > > > > + std::vector<llvm::StringRef> args_ref;
> > > > > > + for (auto &entry : args.entries())
> > > > > > + args_ref.push_back(entry.ref);
> > > > > > +
> > > > > > + // TODO: only flatten raw input.
> > > > > > + // Inputs from lldb console through the stdin are not raw, for
> > > > > > example,
> > > > > > + // A command line like "dir c:\" is attained as "dir c":\\".
> > > > > > Trying to flatten such
> > > > > > + // input will result in unexpected errors. In this case, the
> > > > > > flattend string will be
> > > > > > + // interpreted as "dir c:\\ which is a wrong usage of `dir`
> > > > > > command.
> > > > > > +
> > > > > > + command = llvm::sys::flattenWindowsCommandLine(args_ref);
> > > > > > + return true;
> > > > > > }
> > > > > > +} // namespace
> > > > > >
> > > > > I am afraid you lost me here. By the time something gets to this
> > > > > function, it has already been parsed into individual argv strings. I
> > > > > am not sure which ones do you consider raw, or why should it make a
> > > > > difference. (lldb builtin interpreter has a concept of "raw"
> > > > > commands, but I don't think that's what you mean here)
> > > > >
> > > > > This function should take those argv strings, no matter what they
> > > > > are, and recompose them into a single string. If those strings are
> > > > > wrong, then it's output will also be wrong, but that's not a problem
> > > > > of this function -- it's a problem of whoever parsed those strings.
> > > > >
> > > > > I won't have access to a windows machine this week to check this out,
> > > > > but I can take a look at that next week. In the mean time, I would be
> > > > > fine with just xfailing the single test which fails because of this.
> > > > > I think that's a good tradeoff, as this would fix a bunch of other
> > > > > tests as well as unblock you on your lldb-server work.
> > > > I can't test this out, but the place I'd expect the problem to be is in
> > > > `ProcessLaunchInfo::ConvertArgumentsForLaunchingInShell`. This function
> > > > seems to be blissfully unaware of the windows quoting rules, and yet
> > > > it's the one that turns `platform shell dir c:\` into `cmd`, `/c`
> > > > `whatever`. The flatten function then gets this argv array and flattens
> > > > it one more time. I am not sure if that's the right thing to do on
> > > > windows.
> > > It is observed when you type "dir c:\" on lldb console, the
> > > IOHandlerEditline::GetLine will yield "dir c:\\" for you through the
> > > standard input (I skipped the new line char '\n'). And the
> > > CommandInterpreter::ResolveCommandImpl won't get the raw command line
> > > string, i mean "dir c:\", even I force WantsRawCommandString() true to
> > > get one.
> > How are you observing that? Are you sure that's what happens, and it's not
> > just the lldb formatting of c strings (when lldb displays a `const char *`
> > variable, it will automatically escape any backslashes within, which means
> > the backslashes will appear doubled).
> >
> > I'd be surprised if extra backslashes appear at this level (they certainly
> > don't on linux), as plenty of other things would fail if this didn't work.
> I set the breakpoint through MSVC, triggered it by typing the command "dir
> c:\" on lldb console and the value I read was "dir c:\\\n".
Isn't MSVC doing the same thing here? Otherwise, how would you be able to tell
a line feed character ("\n") from a literal backslash followed by the letter N?
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D56230/new/
https://reviews.llvm.org/D56230
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
