[Lldb-commits] [PATCH] D26883: Demonstrate proposed new Args API

[Pavel Labath via lldb-commits]

labath added a comment.

> Assuming we do that, what interface do you think would be simpler? We still
>  need easy access to both a StringRef and a c_str(), since StringRef::data
>  is not guaranteed to be null terminated, so the entry thing is still nice.

I was assuming (possibly incorrectly, I did not look at that code much) that the main user of the null-terminated string version was execve(2), which needs an entire list of strings, not just a single item, in which case we could have the iteration simply be over StringRefs. That said, it probably does not matter now, as judging by your comments, you're looking for incremental changes, not one grand final design (btw, I didn't mean to suggest that this should all be done in one patch). In that case, this looks fine as far as I am concerned. We can always revise this later, and it doesn't look like it will require any major rewrite, just a bit of syntax-twiddling. You can then read my comments as "the direction I would like to move us in".

> One idea might be to have the entry contain 2 StringRefs. `str` and
> `quoted_str`. This way you never get access to the underlying quote char,
>  just the full arg, either quoted or unquoted (although doing this would
>  still be better done orthogonally to this patch)

I don't think this is a good idea, as I don't see a need to be able to access the original quoted string this way. Also, when you construct the args vector programmatically, you would have to invent a quoted representation without knowing if it will ever be used. I'd prefer to have a standalone quote function instead, as then it can be used in other contexts as well (separate algorithms from data).


https://reviews.llvm.org/D26883