__need macros for communication with compiler-provided headers

Message ID CAKCAbMj-V1bq5VM=a+WV0F6cv+NVf9cH3P+V6zupRRAfmn-oGg@mail.gmail.com
State Superseded
Headers

Commit Message

Zack Weinberg March 24, 2017, 11:55 a.m. UTC
  On Thu, Mar 23, 2017 at 11:38 AM, Chris Metcalf <cmetcalf@mellanox.com> wrote:
> On 3/22/2017 2:47 PM, Zack Weinberg wrote:
>>
>> So my suggestion would be that you provide an uapi/arch/int_reg_t.h
>> (or something like that; it doesn't matter what the name is) that
>> _only_  defines int_reg_t (and names in the implementation namespace)
>> and then the glibc headers that need int_reg_t can include that.
>> You'll find, when you make this change, that arch/abi.h is suddenly
>> quite a bit tidier.
>>
>> The catch though is that we do need a transition plan.  I suspect it's
>> not a big deal if old libc headers + new kernel headers start getting
>> _all_  of arch/abi.h from the small number of headers that use the
>> __need macro, so maybe it can be as simple as "as of version X.Y of
>> glibc we require version A.B.C or later kernel headers for Tile."
>
>
> My concern is that we don't want to screw up the namespace for anything
> that includes <bits/link.h> or <bits/setjmp.h> due to getting all of
> <arch/abi.h>.
>
> How does this change look to you?

I don't know enough about how these files are used outside of glibc to
do a full review, but that looks like it will work from our end.  The
change we would make is simply

plus a bump to the minimum required kernel-header version for Tile.
(This is independent of the minimum required *running kernel*
version.)  Obviously we would have to wait for arch/intreg.h to be in
a released kernel.

zw
  

Comments

Chris Metcalf March 27, 2017, 5:57 p.m. UTC | #1
On 3/24/2017 7:55 AM, Zack Weinberg wrote:
> On Thu, Mar 23, 2017 at 11:38 AM, Chris Metcalf <cmetcalf@mellanox.com> wrote:
>> On 3/22/2017 2:47 PM, Zack Weinberg wrote:
>>> So my suggestion would be that you provide an uapi/arch/int_reg_t.h
>>> (or something like that; it doesn't matter what the name is) that
>>> _only_  defines int_reg_t (and names in the implementation namespace)
>>> and then the glibc headers that need int_reg_t can include that.
>>> You'll find, when you make this change, that arch/abi.h is suddenly
>>> quite a bit tidier.
>>>
>>> The catch though is that we do need a transition plan.  I suspect it's
>>> not a big deal if old libc headers + new kernel headers start getting
>>> _all_  of arch/abi.h from the small number of headers that use the
>>> __need macro, so maybe it can be as simple as "as of version X.Y of
>>> glibc we require version A.B.C or later kernel headers for Tile."
>>
>> My concern is that we don't want to screw up the namespace for anything
>> that includes <bits/link.h> or <bits/setjmp.h> due to getting all of
>> <arch/abi.h>.
>>
>> How does this change look to you?
> I don't know enough about how these files are used outside of glibc to
> do a full review, but that looks like it will work from our end.

I mailed out a patch to LKML and staged the change in linux-tile
to go in during the merge window for 4.12.
  

Patch

diff --git a/sysdeps/tile/bits/link.h b/sysdeps/tile/bits/link.h
index d29725892e..7d03ae2d9f 100644
--- a/sysdeps/tile/bits/link.h
+++ b/sysdeps/tile/bits/link.h
@@ -20,8 +20,7 @@ 
 # error "Never include <bits/link.h> directly; use <link.h> instead."
 #endif

-#define __need_int_reg_t
-#include <arch/abi.h>
+#include <arch/intreg.h>


 /* Registers for entry into PLT.  */
diff --git a/sysdeps/tile/bits/setjmp.h b/sysdeps/tile/bits/setjmp.h
index e9efc3b5ef..3123f74752 100644
--- a/sysdeps/tile/bits/setjmp.h
+++ b/sysdeps/tile/bits/setjmp.h
@@ -26,8 +26,7 @@ 

 #ifndef _ASM

-#define __need_int_reg_t
-#include <arch/abi.h>
+#include <arch/intreg.h>

 typedef __uint_reg_t __jmp_buf[32];

diff --git a/sysdeps/unix/sysv/linux/tile/sys/procfs.h
b/sysdeps/unix/sysv/linux/tile/sys/procfs.h
index ce3e33ddef..3fb3bc0dff 100644
--- a/sysdeps/unix/sysv/linux/tile/sys/procfs.h
+++ b/sysdeps/unix/sysv/linux/tile/sys/procfs.h
@@ -32,8 +32,7 @@ 
 #include <sys/time.h>
 #include <sys/types.h>

-#define __need_int_reg_t
-#include <arch/abi.h>
+#include <arch/intreg.h>

 __BEGIN_DECLS