labath added a comment.
Thanks for updating this patch. This seems ok to me (modulo comments here and
the other patch), but I think it would be better to move all of the SB changes
into that other patch. (the reason I requested this update was to see whether
the other patch has all that's needed to implement the given functionality --
and I suspected there would be some bits missing :))
================
Comment at: lldb/bindings/interface/SBLaunchInfo.i:65
void
- SetEnvironmentEntries (const char **envp, bool append);
+ SetEnvironmentEntries (const char * const*envp, bool append);
+
----------------
This is going to change the mangled name of the function. Either add a new
overload or `const_cast` at the call site. (But if you implement the other
comment, neither of those will be needed).
================
Comment at: lldb/include/lldb/API/SBEnvironment.h:62
+ lldb_private::Environment &GetInnerEnvironment() const;
+
----------------
Usually, these functions are called `get()` (when they return a pointer) or
`ref()` (for a reference).
================
Comment at: lldb/source/API/SBLaunchInfo.cpp:198
+ (const lldb::SBEnvironment &, bool), env, append)
+ SetEnvironmentEntries(env.GetInnerEnvironment().getEnvp().get(), append);
+}
----------------
This is a pretty long-winded way of implementing this functionality, as the
first thing the other function will do is recreate an Environment object. It
would be better to have the other function redirect to this one.
Repository:
rG LLVM Github Monorepo
CHANGES SINCE LAST ACTION
https://reviews.llvm.org/D74636/new/
https://reviews.llvm.org/D74636
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
https://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
