clayborg requested changes to this revision.
clayborg added a comment.
This revision now requires changes to proceed.
I would rather you be able to ask the current platform for the default memory
cache line size and have lldb_private::Process do this automatically on the
first memory read if the process property wasn't manually set. So my inline
comments will reflect this notion.
================
Comment at: include/lldb/Target/Process.h:70-72
@@ -69,2 +69,5 @@
+ lldb::OptionValueSP
+ GetMemoryCacheLineSizeOption ();
+
Args
----------------
Remove
================
Comment at:
source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp:148
@@ +147,3 @@
+ lldb::ProcessSP process_sp =
PlatformRemoteGDBServer::DebugProcess(launch_info, debugger, target, error);
+ AdjustProcessProperties(process_sp);
+ return process_sp;
----------------
Remove the call to AdjustProcessProperties(...) and let the process call
platform_sp->GetDefaultMemoryCacheLineSize() when it needs to. If the only this
this PlatformAndroidRemoteGDBServer::DebugProcess() was doing was to allow
calling AdjustProcessProperties() then remove this whole function.
================
Comment at:
source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp:159
@@ +158,3 @@
+ lldb::ProcessSP process_sp = PlatformRemoteGDBServer::Attach(attach_info,
debugger, target, error);
+ AdjustProcessProperties(process_sp);
+ return process_sp;
----------------
Remove the call to AdjustProcessProperties(...) and let the process call
platform_sp->GetDefaultMemoryCacheLineSize() when it needs to. If the only this
this PlatformAndroidRemoteGDBServer::Attach() was doing was to allow calling
AdjustProcessProperties() then remove this whole function.
================
Comment at:
source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.cpp:163-174
@@ -139,1 +162,14 @@
+
+void
+PlatformAndroidRemoteGDBServer::AdjustProcessProperties(const lldb::ProcessSP
&process_sp)
+{
+ if (! process_sp)
+ return;
+
+ lldb::OptionValueSP value_sp = process_sp->GetMemoryCacheLineSizeOption();
+
+ if (value_sp && ! value_sp->OptionWasSet())
+ value_sp->SetUInt64Value(g_android_default_cache_size);
+}
+
void
----------------
Change this to be:
```
uint32_t
PlatformAndroidRemoteGDBServer:: GetDefaultMemoryCacheLineSize()
{
return g_android_default_cache_size;
}
```
Add the following to Platform.h inside the lldb_private::Platform class
definition:
```
virtual uint32_t GetDefaultMemoryCacheLineSize() { return 0; }
```
================
Comment at:
source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h:39-49
@@ -38,1 +38,13 @@
+ lldb::ProcessSP
+ DebugProcess(ProcessLaunchInfo &launch_info,
+ Debugger &debugger,
+ Target *target, // Can be NULL, if NULL create a new
target, else use existing one
+ Error &error) override;
+
+ lldb::ProcessSP
+ Attach(ProcessAttachInfo &attach_info,
+ Debugger &debugger,
+ Target *target, // Can be NULL, if NULL create a new target,
else use existing one
+ Error &error) override;
+
----------------
If the only this these fucntions were added for was to allow calling
AdjustProcessProperties() then remove these.
================
Comment at:
source/Plugins/Platform/Android/PlatformAndroidRemoteGDBServer.h:55-56
@@ -42,1 +54,4 @@
+ void
+ AdjustProcessProperties(const lldb::ProcessSP &process_sp);
+
----------------
Change to be:
```
uint32_t GetDefaultMemoryCacheLineSize() override;
```
================
Comment at: source/Target/Process.cpp:183-189
@@ -182,1 +182,9 @@
+lldb::OptionValueSP
+ProcessProperties::GetMemoryCacheLineSizeOption()
+{
+ const uint32_t idx = ePropertyMemCacheLineSize;
+ lldb_private::ExecutionContext exe_ctx(m_process);
+ return m_collection_sp->GetPropertyAtIndex(&exe_ctx, true,
idx)->GetValue();
+}
+
----------------
Remove this and modify Process to fetch this information on attach if the
process option wasn't set. This means you don't need to expose this
OptionValueSP and you can do it all within the process by grabbing the platform
from the target and then calling platform_sp->GetDefaultMemoryCacheLineSize().
The default platform implementation should return 0 and if zero is returned
(meaning no preference), then continue using the current setting that process
was using from the setting, else override the current value.
http://reviews.llvm.org/D13812
_______________________________________________
lldb-commits mailing list
lldb-commits@lists.llvm.org
http://lists.llvm.org/cgi-bin/mailman/listinfo/lldb-commits
