[Lldb-commits] [PATCH] D56232: [lldb-server] Add remote platform capabilities for Windows

Hui Huang via Phabricator via lldb-commits lldb-commits at lists.llvm.org
Fri Jan 11 16:53:23 PST 2019

Hui added a comment.

In D56232#1349407 <https://reviews.llvm.org/D56232#1349407>, @labath wrote:

> In D56232#1348514 <https://reviews.llvm.org/D56232#1348514>, @zturner wrote:
> > Is that even necessary?  If a platform is not remote aware, `IsHost()` will always just return `true` by definition.  So we could put all of this in the existing `Platform` base class.
> I remember looking at this a while a go and concluding against it, but i'm not sure if it was impossible of just I didn't like the result.
> The issue here is that PlatformWindows and PlatformPosix already have a m_remote_platform member (which normally is an instance of PlatformRemoteGDBServer). To move the common class into the base one, we'd need to move this member too. That would mean that any platform has a "remote" member, even those that already are "remote". That sounds a bit weird.

Yes. I think the thing is the existing design makes PlatformWindows plugin play dual roles: a "host" and "remote-windows" platform which simplily is a pass-through to PlatformRemoteGDBServer.  Not quite sure about such intent of design.  Maybe to avoid creating new plugin for a remote platform or just to simplifying the init/release in plugin manager.  To break them apart, adding back the plugin, maybe PlatformRemoteWindows

PlatformWindows:  host implementation only. Most common parts will go to the platform base.

PlatformRemoteWindows: for communicating with remote only.  Could just be RemoteAwarePlatform




More information about the lldb-commits mailing list