[Lldb-commits] [PATCH] D29266: Synchronize PlatformNetBSD with Linux

[Pavel Labath via Phabricator via lldb-commits]

labath added a comment.

After looking at this closer, I don't think we will even need a separate class to achieve deduplication. The code is so heavily duplicated (see comments on the first three non-boilerplate functions, I did not look at the rest yet, but I expect the situation to be the same) already that we can just push the definitions down the existing class hierarchy and achieve massive savings that way. After we're done with that, I think we'll find that the PlatformLinux class will be so small that your current s/linux/netbsd/ approach will no longer be a problem.

If @clayborg agrees with this approach, I can help you prepare the ground for that.



================
Comment at: source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp:200
 
 Error PlatformNetBSD::ResolveExecutable(
+    const ModuleSpec &ms, lldb::ModuleSP &exe_module_sp,
----------------
PlatformLinux, PlatformFreeBSD and PlatformWindows have copies of this code already. Proposal:
- put this code into `Platform::ResolveExecutable`
- remove other identical copies


================
Comment at: source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp:325
 
-// From PlatformMacOSX only
 Error PlatformNetBSD::GetFileWithUUID(const FileSpec &platform_file,
+                                     const UUID *uuid_ptr,
----------------
Every platform except `PlatformDarwin` already defines this function to the exact same code. Proposal:
- put this code into `Platform::GetFileWithUUID`
- remove other definitions with the same code


================
Comment at: source/Plugins/Platform/NetBSD/PlatformNetBSD.cpp:354
 
 bool PlatformNetBSD::GetProcessInfo(lldb::pid_t pid,
+                                   ProcessInstanceInfo &process_info) {
----------------
Every platform already defines this function to the same code. Proposal:
- move this code into `Platform::GetProcessInfo`
- remove other (*all*) identical definitions


Repository:
  rL LLVM

https://reviews.llvm.org/D29266