labath added a comment.
Thanks for the heads up. I can test the android part on Wednesday, but right
now I don't see a reason why that shouldn't work there.
Overall, I like the idea of using the process class for caching some platform
resources. If we end up needing to do that more often, we can think of making
using some more generic container for that instead of making a pair of
getters/setters for each value.
Apart from the low-level inline comments, the one high-level comment I have is
that the `DoLoadImage` function is getting a bit big. I think it would help
readability to split it up a bit. At least the
get-cached-function-or-build-one-if-it-is-not-present part seems like it could
be easily separated out into a helper function.
================
Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1036-1037
+ // We made a good utility function, so cache it in the process:
+ dlopen_utility_func = dlopen_utility_func_up.release();
+ process->SetLoadImageUtilityFunction(dlopen_utility_func);
+ arguments = do_dlopen_function->GetArgumentValues();
----------------
If `SetLoadImageUtilityFunction` takes a unique_ptr then you can do a
`SetLoadImageUtilityFunction(std::move(dlopen_utility_func_up))` here.
================
Comment at: source/Target/Process.cpp:6251
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+ if (platform != GetTarget().GetPlatform().get())
+ return nullptr;
----------------
Will this ever be true? I would not expect one to be able to change the
platform of a process mid-session.
================
Comment at: source/Target/Process.cpp:6256
+
+void Process::SetLoadImageUtilityFunction(UtilityFunction *utility_func) {
+ m_dlopen_utility_func_up.reset(utility_func);
----------------
I think the argument of this function should be a unique_ptr as well.
Repository:
rL LLVM
https://reviews.llvm.org/D45703
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
