[Lldb-commits] [PATCH] D49207: Get rid of the C-string parameter in DoExecute

Jim Ingham via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Wed Jul 11 16:34:51 PDT 2018


jingham requested changes to this revision.
jingham added a comment.
This revision now requires changes to proceed.

I think the CommandObjectRegex check that you took out wasn't something devious or clever, but was just an incomplete (and so buggy) version of the check:

if (command && command[0])

that you've been replacing with:

if (command.empty())

everywhere else in the patch.  After all, the error message if you take the failing branch was:

  result.AppendError("empty command passed to regular expression command");

So I think instead of taking this line out, you should also convert it to an if(command.empty()) and early return it with the error message you removed.

Other than that, this change looks fine.


https://reviews.llvm.org/D49207





More information about the lldb-commits mailing list