[Lldb-commits] [PATCH] Make llgs build on Android. No functionality change.

[Zachary Turner]

I proposed one solution above.  Define the method in HostInfoLinux, then
check for #if defined(__ANDROID__) and assert if it's true, otherwise
delegate to the base class via HostInfoPosix::Method().

Jim suggested another approach, which is to make HostInfoAndroid, and only
override the methods that android cares about, and update the typedef in
HostInfo.h.  The latter yields cleaner code, but is a bit more work.  I'm
willing to do that extra work if Tong just wants to get the change in.  But
moving the preprocessor checks to HostInfoLinux should be pretty simple.

Looking over the patch, all of the android-specific stuff is basically
"fire this assertion or return an error if android, otherwise use the
existing behavior".  I think all of those use cases work cleanly with the
current "static inheritance" model.

On Fri, Sep 26, 2014 at 1:36 PM, Todd Fiala <tfiala at google.com> wrote:

> I think some of the changes you wanted to see Tong make (which I think are
> fine to change) are what Tong's looking at now, which need to deviate some
> POSIX code in very minor ways.
>
> Tong - can you call out the code for us?
>
> Thanks,
> Todd
>
> On Fri, Sep 26, 2014 at 1:34 PM, Zachary Turner <zturner at google.com>
> wrote:
>
>> I think that for the purposes that I wrote HostInfo to handle, very
>> simple methods that answer simple queries about the host os, the virtual
>> methods would not be needed.  For example, what's the username of the
>> currently logged in user?  What's the page size?  Get the value of an
>> environment variable.  If something is more complicated than that, it
>> probably belongs somewhere else.  Do you have an example in mind that you
>> think is a natural fit for HostInfo, but would require calling virtual
>> methods?
>>
>> On Fri, Sep 26, 2014 at 1:32 PM, Todd Fiala <tfiala at google.com> wrote:
>>
>>> >It wasn't made a class instance hierarchy for the same reason that
>>> Host wasn't a class instance to begin with.  HostInfo only contains method
>>> that are inherently static.  You could make it an instance, but it would be
>>> awkward, because you would have to create one for the sole purpose of
>>> calling a method that is actually static.
>>>
>>> Right - but when we want to nuke #ifdefs, we really do need the virtual
>>> methods.  Which I think is all for the best.  But needs the virtual methods
>>> to share large bits of code and only deviate in little places.
>>>
>>> Is there any big reason not to just go to a singleton instance here?
>>>
>>> On Fri, Sep 26, 2014 at 1:30 PM, Zachary Turner <zturner at google.com>
>>> wrote:
>>>
>>>> It wasn't made a class instance hierarchy for the same reason that Host
>>>> wasn't a class instance to begin with.  HostInfo only contains method that
>>>> are inherently static.  You could make it an instance, but it would be
>>>> awkward, because you would have to create one for the sole purpose of
>>>> calling a method that is actually static.
>>>>
>>>> On Fri, Sep 26, 2014 at 1:17 PM, Todd Fiala <tfiala at google.com> wrote:
>>>>
>>>>> Hey Zachary,
>>>>>
>>>>> I just started looking at the HostInfo* classes.  I'm a little
>>>>> confused - we are going to want to have Android behavior differ in just a
>>>>> tiny little way from Linux (and POSIX).  So we're going to want to break
>>>>> out some implementation details into some virtual methods that are possibly
>>>>> no-op or different-op on the *largely same* code for POSIX/Linux.  Yet all
>>>>> the calls in these classes are static.
>>>>>
>>>>> Why isn't this just a class instance hierarchy?  We're going to want
>>>>> it to be more like that so we can use typical C++ class design patterns
>>>>> like using virtual methods and the like.
>>>>>
>>>>> Or did you have some other paradigm in mind for nearly-similar classes
>>>>> that want to share the bulk of the code in these classes?
>>>>>
>>>>> On Fri, Sep 26, 2014 at 1:09 PM, Todd Fiala <tfiala at google.com> wrote:
>>>>>
>>>>>> >>! In D5495#27, @zturner wrote:
>>>>>> > I'm not seeing the changes to HostInfoPosix and HostThreadPosix
>>>>>> that I
>>>>>> > suggested.  Are those still coming in a followup?
>>>>>>
>>>>>> With Android, it's *mostly* Linux (at the kernel level) except we
>>>>>> have a whole slew of different runtime libraries that differ.  So, for much
>>>>>> of the behavior, it will be like Linux/POSIX, but with certain libc calls
>>>>>> and whatnot that just don't exist.  Ideally we minimize how much we differ
>>>>>> and only when needed.
>>>>>>
>>>>>> Tong, can you take a look at that?  If we need a Linux-derived
>>>>>> Android variant of these classes, this might be a good time to look at
>>>>>> those.  Thanks.
>>>>>>
>>>>>> > (paraphrased) how do you launch on Android, then?
>>>>>>
>>>>>> In general you don't.  The Android zygote takes care of launching for
>>>>>> userland, and debuggers usually run attach-only.
>>>>>>
>>>>>> That's just a half-truth, though.  We do have low-level, internal
>>>>>> capabilities for launching,  but since that is not a supported path for
>>>>>> developers, they are not exposed in a way we can access with POSIX-like
>>>>>> calls.  The launch code might end up only showing up in an AOSP environment
>>>>>> but, in any event, the attach path is the crucial one for Android right now.
>>>>>>
>>>>>> http://reviews.llvm.org/D5495
>>>>>>
>>>>>>
>>>>>>
>>>>>
>>>>>
>>>>> --
>>>>> Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
>>>>>
>>>>
>>>>
>>>
>>>
>>> --
>>> Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
>>>
>>
>>
>
>
> --
> Todd Fiala | Software Engineer | tfiala at google.com | 650-943-3180
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/lldb-commits/attachments/20140926/14ce3fe4/attachment.html>