zturner added inline comments.
================
Comment at: source/Interpreter/Args.cpp:286-288
@@ -338,5 +285,5 @@
-const char **Args::GetConstArgumentVector() const {
- if (!m_argv.empty())
- return const_cast<const char **>(&m_argv[0]);
- return nullptr;
+ // This is kind of gross, but it ensures that if someone passes args.data()
to
+ // a function which expects an array terminated with a null entry, it will
+ // work.
+ result.push_back(nullptr);
----------------
zturner wrote:
> clayborg wrote:
> > zturner wrote:
> > > clayborg wrote:
> > > > Remove this comment. The whole point off this function is to get an
> > > > argument vector that is NULL terminated. Feel free to rename the
> > > > function as desired to indicate this, but that is the whole point of
> > > > this function. Feel free to document it if needed as well.
> > > There are two ways we use these argument vectors. One is in a call to a
> > > function which takes an argc and an argv. In that case the null
> > > terminator is unnecessary because you're specifying the argc explicitly.
> > > Another is in a call to a function which you do not specify an argc, and
> > > in tht case it requires it to be null terminated.
> > >
> > > The point of this function isn't "return me something that's null
> > > terminated", it's "return me something I can use like an argv". That
> > > doesn't require null termination. And in fact, if you look at the
> > > callsites I fixed up, not a single one depends on the null termination.
> > > Everyone just needs to know the size. And for that you can call
> > > `result.size()`. If you add the null terminator, then you have to write
> > > `result.size()-1`, which is unnecessarily confusing. I believe it
> > > provides the most logical behavior as it is currently written.
> > Remove the comment. Even in "main(int argc, const char **argv)" the "argv"
> > is null terminated. So all unix variants expect the null termination. The
> > correct solution is to add a boolean argument:
> >
> > ```
> > std::vector<const char *> Args::GetArgumentVector(bool null_terminate =
> > true) const {
> > ```
> >
> > Then it is clear.
> Even in main though, the null is not counted in the argc, right? So as
> written it will conform to the semantics of main. `argc` is the number of
> non-null pointers, but the null is still there anyway. If you add the
> boolean argument and pass true, it will give you back something whose `argc`
> (i.e. `result.size()`) is one too high.
BTW, `llvm::SmallString` uses this same trick to return a null terminated
string without modifying the size. Look at the implementation of
`SmallString::c_str()` in `SmallString.h`
https://reviews.llvm.org/D24952
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
