Re: [Buildroot] [PATCH 2/2] package/gdb: use stat() privided by the system

[Pedro Alves <palves@redhat.com>]

On 09/11/2018 07:21 PM, Sergio Durigan Junior wrote:
> On Tuesday, September 11 2018, Pedro Alves wrote:
> 
>> On 09/11/2018 01:38 AM, Sergio Durigan Junior wrote:
>>
>>> This is happening because, before the commit mentioned above,
>>> 'common-utils.c' (which gets transformed into 'common-utils-ipa.c'
>>> during the gdbserver build) wasn't calling 'stat'.  It doesn't seem like
>>> a regression; it seems like a hidden problem that was uncovered by the
>>> need of 'stat'.
>>>
>>> I don't know why this problem is manifesting only when compiling IPA,
>>> and not when compiling 'common-utils.c' during GDB's/gdbserver's build.
>>
>> Because the IPA doesn't link with gnulib.  And the answer to that wouldn't
>> be as simple as "just link it in", because the IPA objects are supposed
>> to be compiled with -fPIC and -fvisibility=hidden.  So we'd need a third
>> build of gnulib for the IPA.
> 
> That explains it.  It seems strange to me that we still include the
> gnulib headers when compiling IPA; I confess I just assumed IPA was
> linking with gnulib because of this.

In order _not_ to include the gnulib headers, that would mean that we'd
be OK with going back to making sure we do any necessary portability
autoconf/#ifdefery ourselves in all of common and gdbserver code that is
used by the IPA, which doesn't sound very appealing to me.  
I'd think it better / simpler maintenance-wise going forward, to work in the
direction of linking with gnulib if/when we find a need.  

The IPA has so few dependencies by design that in practice just using the
headers should be fine, with gnulib ironing-out any header-only portability
issues.  It's quite likely that a port that can use the IPA won't really need
any gnu_foo replacement function.  And if it does, then it should be
better to use gnulib's replacements instead of duplicating what gnulib
already does, I'd think.

> 
>> It doesn't seem like this code that calls stat (is_regular_file?) is
>> useful for the IPA, so a quicker/simpler fix would be to simply move
>> that function out of common-utils.c into some other file that is not
>> shared with the IPA.
> 
> It's not useful for IPA; it could be moved to common/filestuff.c, for
> example.  

Right.

> But IMHO, we should probably have an explicit file just for
> IPA, because otherwise we'll forget about this restriction and re-add
> some 'stat' calls to common-utils.c.

I'm afraid I don't understand what you mean here.

Thanks,
Pedro Alves