[2/2] Revert "linux: Move xmknod{at} to compat symbols"

  This reverts commit 81b83ff61f95f30ad53d6075247af0ea61a0b16e responsible
to make both __xmknod{at} compatible symbols.

The idea is to not force static libraries built against old glibc
to update against new glibcs (since they reference the the
xmknod{at} symbols).
---
 sysdeps/mach/hurd/xmknod.c         | 11 -----------
 sysdeps/mach/hurd/xmknodat.c       | 11 -----------
 sysdeps/unix/sysv/linux/xmknod.c   |  3 ---
 sysdeps/unix/sysv/linux/xmknodat.c |  2 --
 4 files changed, 27 deletions(-)
  

* Adhemerval Zanella:

> This reverts commit 81b83ff61f95f30ad53d6075247af0ea61a0b16e responsible
> to make both __xmknod{at} compatible symbols.
>
> The idea is to not force static libraries built against old glibc
> to update against new glibcs (since they reference the the
> xmknod{at} symbols).

I think it's not correct to label this as a revert in the Git sense.
At best it's a partial revert.  I tried first to verify this as an
actual revert and was wondering if I got the diffing wrong.

I think you need to remove libc_hidden_def and libc_hidden_proto around
__xmknod, __xmknodat.  “git grep 'libc_hidden.*__xmknod'” should not
return anything.  Maybe that should be a separate patch.

Thanks,
Florian
  

On 23/10/2020 13:35, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> This reverts commit 81b83ff61f95f30ad53d6075247af0ea61a0b16e responsible
>> to make both __xmknod{at} compatible symbols.
>>
>> The idea is to not force static libraries built against old glibc
>> to update against new glibcs (since they reference the the
>> xmknod{at} symbols).
> 
> I think it's not correct to label this as a revert in the Git sense.
> At best it's a partial revert.  I tried first to verify this as an
> actual revert and was wondering if I got the diffing wrong.

Ok, I will change to a partial remove.

> 
> I think you need to remove libc_hidden_def and libc_hidden_proto around
> __xmknod, __xmknodat.  “git grep 'libc_hidden.*__xmknod'” should not
> return anything.  Maybe that should be a separate patch.

If I recall correctly they are still required for Hurd, but I double
check.  In fact I think we should remove all the io/*xstat* and
io/*xmknod* since there is no point in providing them as generic
interfaces.
  

* Adhemerval Zanella:

> On 23/10/2020 13:35, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> This reverts commit 81b83ff61f95f30ad53d6075247af0ea61a0b16e responsible
>>> to make both __xmknod{at} compatible symbols.
>>>
>>> The idea is to not force static libraries built against old glibc
>>> to update against new glibcs (since they reference the the
>>> xmknod{at} symbols).
>> 
>> I think it's not correct to label this as a revert in the Git sense.
>> At best it's a partial revert.  I tried first to verify this as an
>> actual revert and was wondering if I got the diffing wrong.
>
> Ok, I will change to a partial remove.
>
>> 
>> I think you need to remove libc_hidden_def and libc_hidden_proto around
>> __xmknod, __xmknodat.  “git grep 'libc_hidden.*__xmknod'” should not
>> return anything.  Maybe that should be a separate patch.
>
> If I recall correctly they are still required for Hurd, but I double
> check.  In fact I think we should remove all the io/*xstat* and
> io/*xmknod* since there is no point in providing them as generic
> interfaces.

Then I think you should add the missing libc_hidden_defs instead (to the
Linux implementations).  The present state is inconsistent and
potentially brittle.

Thanks,
Florian
  

On 23/10/2020 13:43, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 23/10/2020 13:35, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>> This reverts commit 81b83ff61f95f30ad53d6075247af0ea61a0b16e responsible
>>>> to make both __xmknod{at} compatible symbols.
>>>>
>>>> The idea is to not force static libraries built against old glibc
>>>> to update against new glibcs (since they reference the the
>>>> xmknod{at} symbols).
>>>
>>> I think it's not correct to label this as a revert in the Git sense.
>>> At best it's a partial revert.  I tried first to verify this as an
>>> actual revert and was wondering if I got the diffing wrong.
>>
>> Ok, I will change to a partial remove.
>>
>>>
>>> I think you need to remove libc_hidden_def and libc_hidden_proto around
>>> __xmknod, __xmknodat.  “git grep 'libc_hidden.*__xmknod'” should not
>>> return anything.  Maybe that should be a separate patch.
>>
>> If I recall correctly they are still required for Hurd, but I double
>> check.  In fact I think we should remove all the io/*xstat* and
>> io/*xmknod* since there is no point in providing them as generic
>> interfaces.
> 
> Then I think you should add the missing libc_hidden_defs instead (to the
> Linux implementations).  The present state is inconsistent and
> potentially brittle.

Why do we need to add the libc_hidden_defs for Linux? Currently they are not
used internally anymore (both xmknod and xmknodat will call __mknodat).

I think it would be better to check if the hidden_proto/def are stil required
for Hurd and it were to move the libc_hidden_def to a Hurd only header.
  

* Adhemerval Zanella:

>>> If I recall correctly they are still required for Hurd, but I double
>>> check.  In fact I think we should remove all the io/*xstat* and
>>> io/*xmknod* since there is no point in providing them as generic
>>> interfaces.
>> 
>> Then I think you should add the missing libc_hidden_defs instead (to the
>> Linux implementations).  The present state is inconsistent and
>> potentially brittle.
>
> Why do we need to add the libc_hidden_defs for Linux? Currently they are not
> used internally anymore (both xmknod and xmknodat will call __mknodat).

I think they always have to come in pairs, conceptually.  Some pairings
work by accident.  I think the aliasing could be implemented in the
other direction, then this use case (public symbol, no internal
references) would break, but the other one (unused export, internal
references) would work.

Thanks,
Florian
  

On 23/10/2020 14:18, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>>>> If I recall correctly they are still required for Hurd, but I double
>>>> check.  In fact I think we should remove all the io/*xstat* and
>>>> io/*xmknod* since there is no point in providing them as generic
>>>> interfaces.
>>>
>>> Then I think you should add the missing libc_hidden_defs instead (to the
>>> Linux implementations).  The present state is inconsistent and
>>> potentially brittle.
>>
>> Why do we need to add the libc_hidden_defs for Linux? Currently they are not
>> used internally anymore (both xmknod and xmknodat will call __mknodat).
> 
> I think they always have to come in pairs, conceptually.  Some pairings
> work by accident.  I think the aliasing could be implemented in the
> other direction, then this use case (public symbol, no internal
> references) would break, but the other one (unused export, internal
> references) would work.
For this specific case, the libc_hidden_proto on include/sys/stat.h are
only defined for NO_RTLD_HIDDEN, which is only define for Hurd.  So I
don't think there is the need to add the libc_hidden_def for the Linux
implementation. 

This is also for the xstat symbols.
  

* Adhemerval Zanella:

> On 23/10/2020 14:18, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>>>> If I recall correctly they are still required for Hurd, but I double
>>>>> check.  In fact I think we should remove all the io/*xstat* and
>>>>> io/*xmknod* since there is no point in providing them as generic
>>>>> interfaces.
>>>>
>>>> Then I think you should add the missing libc_hidden_defs instead (to the
>>>> Linux implementations).  The present state is inconsistent and
>>>> potentially brittle.
>>>
>>> Why do we need to add the libc_hidden_defs for Linux? Currently they are not
>>> used internally anymore (both xmknod and xmknodat will call __mknodat).
>> 
>> I think they always have to come in pairs, conceptually.  Some pairings
>> work by accident.  I think the aliasing could be implemented in the
>> other direction, then this use case (public symbol, no internal
>> references) would break, but the other one (unused export, internal
>> references) would work.

> For this specific case, the libc_hidden_proto on include/sys/stat.h are
> only defined for NO_RTLD_HIDDEN, which is only define for Hurd.  So I
> don't think there is the need to add the libc_hidden_def for the Linux
> implementation. 
>
> This is also for the xstat symbols.

Hmm.  I really don't like this.  But we can consider it separately from
the (partial) revert.

Thanks,
Florian
  

On 26/10/2020 09:56, Florian Weimer wrote:
> * Adhemerval Zanella:
> 
>> On 23/10/2020 14:18, Florian Weimer wrote:
>>> * Adhemerval Zanella:
>>>
>>>>>> If I recall correctly they are still required for Hurd, but I double
>>>>>> check.  In fact I think we should remove all the io/*xstat* and
>>>>>> io/*xmknod* since there is no point in providing them as generic
>>>>>> interfaces.
>>>>>
>>>>> Then I think you should add the missing libc_hidden_defs instead (to the
>>>>> Linux implementations).  The present state is inconsistent and
>>>>> potentially brittle.
>>>>
>>>> Why do we need to add the libc_hidden_defs for Linux? Currently they are not
>>>> used internally anymore (both xmknod and xmknodat will call __mknodat).
>>>
>>> I think they always have to come in pairs, conceptually.  Some pairings
>>> work by accident.  I think the aliasing could be implemented in the
>>> other direction, then this use case (public symbol, no internal
>>> references) would break, but the other one (unused export, internal
>>> references) would work.
> 
>> For this specific case, the libc_hidden_proto on include/sys/stat.h are
>> only defined for NO_RTLD_HIDDEN, which is only define for Hurd.  So I
>> don't think there is the need to add the libc_hidden_def for the Linux
>> implementation. 
>>
>> This is also for the xstat symbols.
> 
> Hmm.  I really don't like this.  But we can consider it separately from
> the (partial) revert.

Could we track this with a different patch? I would like to close this
and the xstat issue.
  

* Adhemerval Zanella:

> On 26/10/2020 09:56, Florian Weimer wrote:
>> * Adhemerval Zanella:
>> 
>>> On 23/10/2020 14:18, Florian Weimer wrote:
>>>> * Adhemerval Zanella:
>>>>
>>>>>>> If I recall correctly they are still required for Hurd, but I double
>>>>>>> check.  In fact I think we should remove all the io/*xstat* and
>>>>>>> io/*xmknod* since there is no point in providing them as generic
>>>>>>> interfaces.
>>>>>>
>>>>>> Then I think you should add the missing libc_hidden_defs instead (to the
>>>>>> Linux implementations).  The present state is inconsistent and
>>>>>> potentially brittle.
>>>>>
>>>>> Why do we need to add the libc_hidden_defs for Linux? Currently they are not
>>>>> used internally anymore (both xmknod and xmknodat will call __mknodat).
>>>>
>>>> I think they always have to come in pairs, conceptually.  Some pairings
>>>> work by accident.  I think the aliasing could be implemented in the
>>>> other direction, then this use case (public symbol, no internal
>>>> references) would break, but the other one (unused export, internal
>>>> references) would work.
>> 
>>> For this specific case, the libc_hidden_proto on include/sys/stat.h are
>>> only defined for NO_RTLD_HIDDEN, which is only define for Hurd.  So I
>>> don't think there is the need to add the libc_hidden_def for the Linux
>>> implementation. 
>>>
>>> This is also for the xstat symbols.
>> 
>> Hmm.  I really don't like this.  But we can consider it separately from
>> the (partial) revert.
>
> Could we track this with a different patch?

Yes, that's what I meant.

Thanks,
Florian
  

On 10/23/20 12:33 AM, Adhemerval Zanella via Libc-alpha wrote:
> This reverts commit 81b83ff61f95f30ad53d6075247af0ea61a0b16e responsible
> to make both __xmknod{at} compatible symbols.
> 
> The idea is to not force static libraries built against old glibc
> to update against new glibcs (since they reference the the
> xmknod{at} symbols).

Hi Adhemerval,

If you haven't already (I couldn't find it) could you please push this 
set?  AFAICT from Florian's comments he's OK with the patch and the 
hidden symbols could be a separate change.

Thanks,
Siddhesh
  

On 01/12/2020 22:45, Siddhesh Poyarekar wrote:
> On 10/23/20 12:33 AM, Adhemerval Zanella via Libc-alpha wrote:
>> This reverts commit 81b83ff61f95f30ad53d6075247af0ea61a0b16e responsible
>> to make both __xmknod{at} compatible symbols.
>>
>> The idea is to not force static libraries built against old glibc
>> to update against new glibcs (since they reference the the
>> xmknod{at} symbols).
> 
> Hi Adhemerval,
> 
> If you haven't already (I couldn't find it) could you please push this set?  AFAICT from Florian's comments he's OK with the patch and the hidden symbols could be a separate change.

Alright, I will push both reverts, thanks.