labath added a comment.
I like the idea of leaving some comment to have a record that we discussed
making a more generic storage facility, but I don't think we need to do
anything else right now. I doubt we will see many new clients for this very
soon.
================
Comment at: source/Plugins/Platform/POSIX/PlatformPOSIX.cpp:1166
+ // The dlopen succeeded!
+ if (token != 0x0)
+ return process->AddImageToken(token);
----------------
Use LLDB_INVALID_IMAGE_TOKEN here?
================
Comment at: source/Target/Process.cpp:6251
+UtilityFunction *Process::GetLoadImageUtilityFunction(Platform *platform) {
+ if (platform != GetTarget().GetPlatform().get())
+ return nullptr;
----------------
jingham wrote:
> labath wrote:
> > Will this ever be true? I would not expect one to be able to change the
> > platform of a process mid-session.
> Reading through the code I could not convince myself that it could NOT
> happen. Target::SetPlatform is a function anybody can call at any time. I
> agree that it would be odd to do so, and we might think about prohibiting it
> (Target::SetPlatform could return an error, and forbid changing the Platform,
> if the Target has a live process.)
>
> If everybody agrees that that is a good idea, I can do that and remove this
> check.
I see three call in the codebase to SetPlatform. All of them seem to be in the
initialization code, though some of them seem to happen after the process is
launched (DynamicLoaderDarwinKernel constructor).
So it may not be possible to completely forbid setting the platform this way,
but I think we can at least ban changing the platform once it has already been
set in a pretty hard way (lldb_assert or even assert). I think a lot of things
would currently break if someone started changing the platforms of a running
process all of a sudden.
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
