Re: Definitions of types also as macros

2018-11-10 Thread David Holland
On Fri, Nov 09, 2018 at 01:41:35PM +0100, Kamil Rytarowski wrote:
 > >>  > I wanna do this, looks good?
 > >>
 > >> It should probably use double wings:
 > >>
 > >>  > -#ifndef   int8_t
 > >>  > +#ifndef   _BSD_INT8_T_
 > >>
 > >>+#ifndef   __BSD_INT8_T__
 > > 
 > > In terms of namespace consumption, the single leading underscore
 > > followed by an uppercase letter as used here is sufficient.
 > 
 > We alredy use this syntax, eg. in :
 > 
 > #ifdef  _BSD_TIME_T_
 > typedef _BSD_TIME_T_time_t;
 > #undef  _BSD_TIME_T_
 > #endif
 > 
 > (however here the ifdef is switched for some reason)

Those are older, and work differently. So looking the same isn't
necessarily a feature.

all of it ought to be retconned.

The basic problem is: according to standards, certain typedefs need to
be exposed by any of several headers but not others; and some typedefs
need to be accessible to headers that are not allowed to expose them.
(Crazy as that sounds.) However, any combination or order of #includes
must generate only one typedef as repeated typedefs anger the
compiler.

To make life more exciting, some of these types are also
machine-dependent and most of them need to be shared between userland
and kernel.

The "correct" way to handle this (as in, the worked-out and tested way
based on twenty years of hindsight and having seen and tried various
other less successful ways) is:

(1) Create an exported kernel header, that is not part of any
user-facing API and that will never be, and put all the affected MI
types in it as typedefs with underscore prefixes (like __time_t).
This header file should define nothing outside of the implementation
namespace so it can be included freely anywhere.

(2) Create an exported MD kernel header, ditto, and put all the MD
types in it similarly, one for every port and/or one for every
machine. This header file can also be included freely anywhere.

(3) In all headers subject to namespace restrictions, which includes
most stuff shared between user and kernel, write the header itself
using only the underscore-prefix types. This makes these headers
compilable using only the private exported headers (1) and (2).

(4) For every group of types with a different set of exposure criteria
(there are not so many of these actually) create a header file, that
is not part of any user-facing API and will never be, that has a
conventional cpp guard against multiple inclusion, and put the
typedefs for the non-underscore prefixes of the types in the group in
this header. Include this header in each userland header that is
supposed to expose this group of types, and also in sys/types.h, which
is allowed/supposed to expose everything.

This covers all the standards requirements, minimizes the amount of
cutpaste, and avoids having unexpected #defines floating around
breaking builds. The only drawback is that it requires a place to put
header files that are not part of user-facing APIs, and in particular
such a place that can be shared user/kernel. We don't currently have
such a place in NetBSD.

It also is best used together with a kernel headers setup where shared
user/kernel headers are clearly separated from kernel internal
headers, to minimize the number of places where extra leading
underscores are needed and to keep it clear where those are. We also
don't have this in NetBSD.

This in turn should be fixed, and I have been threatening for years to
fix it once we get rename support in version control ... and maybe
we're finally getting near the point where tha'ts possible.

-- 
David A. Holland
dholl...@netbsd.org


Re: Definitions of types also as macros

2018-11-09 Thread Klaus Klein
On Wed, Nov 07, 2018 at 05:43:49AM +0300, Valery Ushakov wrote:
> On Tue, Nov 06, 2018 at 23:20:16 +0100, Rhialto wrote:
> 
> > On Tue 06 Nov 2018 at 23:19:08 +0300, Valery Ushakov wrote:
> > > Also your change breaks redefining intN_t types with the preprocessor.
> > > E.g.
> > > 
> > > #define uint32_t unsigned long long
> > > #include 
> > > 
> > > is now broken with your change.
> > 
> > But should that really be allowed?
> 
> I have no idea :) But then I saw enough of unholy cpp trickery to
> consider that as a possibility, its legal status notwithstanding.

I'd rather not encourage it.  For the example provided the behaviour
is undefined, but not prohibited. ;-)


- Klaus


Re: Definitions of types also as macros

2018-11-09 Thread Kamil Rytarowski
On 09.11.2018 12:17, Klaus Klein wrote:
> On Thu, Nov 08, 2018 at 09:08:04AM +, David Holland wrote:
>> On Tue, Nov 06, 2018 at 03:07:16PM +, co...@sdf.org wrote:
>>  > I wanna do this, looks good?
>>
>> It should probably use double wings:
>>
>>  > -#ifndef  int8_t
>>  > +#ifndef  _BSD_INT8_T_
>>
>>+#ifndef  __BSD_INT8_T__
> 
> In terms of namespace consumption, the single leading underscore
> followed by an uppercase letter as used here is sufficient.
> 
> 
> - Klaus
> 

We alredy use this syntax, eg. in :

#ifdef  _BSD_TIME_T_
typedef _BSD_TIME_T_time_t;
#undef  _BSD_TIME_T_
#endif

(however here the ifdef is switched for some reason)



signature.asc
Description: OpenPGP digital signature


Re: Definitions of types also as macros

2018-11-09 Thread Klaus Klein
On Thu, Nov 08, 2018 at 09:08:04AM +, David Holland wrote:
> On Tue, Nov 06, 2018 at 03:07:16PM +, co...@sdf.org wrote:
>  > I wanna do this, looks good?
> 
> It should probably use double wings:
> 
>  > -#ifndef   int8_t
>  > +#ifndef   _BSD_INT8_T_
> 
>+#ifndef   __BSD_INT8_T__

In terms of namespace consumption, the single leading underscore
followed by an uppercase letter as used here is sufficient.


- Klaus


Re: Definitions of types also as macros

2018-11-08 Thread David Holland
On Tue, Nov 06, 2018 at 08:11:03AM -0800, John Nemeth wrote:
 > } -#ifndef   int8_t
 > } +#ifndef   _BSD_INT8_T_
 > }  typedef   __int8_tint8_t;
 > } -#define   int8_t  __int8_t
 > } +#define   _BSD_INT8_T_
 > }  #endif
 > 
 >  What's going to define _BSD_INT8_T_ and friends?

Eh? It does.

 >  To me, this looks smells like some kind of gross hack to work around
 > broken software.

...as opposed to the prior version?

We can do this right once we have version control with rename :-)

-- 
David A. Holland
dholl...@netbsd.org


Re: Definitions of types also as macros

2018-11-08 Thread David Holland
On Tue, Nov 06, 2018 at 03:07:16PM +, co...@sdf.org wrote:
 > I wanna do this, looks good?

It should probably use double wings:

 > -#ifndef int8_t
 > +#ifndef _BSD_INT8_T_

   +#ifndef __BSD_INT8_T__

-- 
David A. Holland
dholl...@netbsd.org


Re: Definitions of types also as macros

2018-11-06 Thread Valery Ushakov
On Tue, Nov 06, 2018 at 23:20:16 +0100, Rhialto wrote:

> On Tue 06 Nov 2018 at 23:19:08 +0300, Valery Ushakov wrote:
> > Also your change breaks redefining intN_t types with the preprocessor.
> > E.g.
> > 
> > #define uint32_t unsigned long long
> > #include 
> > 
> > is now broken with your change.
> 
> But should that really be allowed?

I have no idea :) But then I saw enough of unholy cpp trickery to
consider that as a possibility, its legal status notwithstanding.

OTHO, I still maintain that the motivating example for this change was
buggy in principle.  You cannot try to do unholy cpp trickery, do it
wrong and then complain that it doesn't work.

-uwe


Re: Definitions of types also as macros

2018-11-06 Thread Rhialto
On Tue 06 Nov 2018 at 23:19:08 +0300, Valery Ushakov wrote:
> Also your change breaks redefining intN_t types with the preprocessor.
> E.g.
> 
> #define uint32_t unsigned long long
> #include 
> 
> is now broken with your change.

But should that really be allowed? I haven't got the Standard at hand
but I strongly suspect that uint32_t is officially in the namespace of
 and therefore, if you #include it, you as a user shouldn't
mess with uint32_t (or risk nasal daemons).

Maybe an implementation something like this could work:

#ifndef uint32_t
typedef __uint32_t  uint32_t;
#define uint32_tuint32_t
#endif

Recursive macro definitions are not supposed to be re-expanded, so this
would work as a guard but still leave uint32_t in source code unchanged.

> -uwe
-Olaf.
-- 
___ Olaf 'Rhialto' Seibert  -- "What good is a Ring of Power
\X/ rhialto/at/falu.nl  -- if you're unable...to Speak." - Agent Elrond


signature.asc
Description: PGP signature


Re: Definitions of types also as macros

2018-11-06 Thread Valery Ushakov
[I missed this thread, so I'm reposting my reply that I originally
sent to source-changes-d]

On Tue, Nov 06, 2018 at 16:38:06 +, co...@sdf.org wrote:

> On Tue, Nov 06, 2018 at 08:11:03AM -0800, John Nemeth wrote:
> > On Nov 6,  3:07pm, co...@sdf.org wrote:
> > }
> > } I wanna do this, looks good?
> > 
> >  No.
> > 
> > } Index: stdint.h
> > } ===
> > } RCS file: /cvsroot/src/sys/sys/stdint.h,v
> > } retrieving revision 1.7
> > } diff -u -r1.7 stdint.h
> > } --- stdint.h  22 Apr 2013 21:26:48 -  1.7
> > } +++ stdint.h  4 Nov 2018 09:35:54 -
> > } @@ -35,54 +35,54 @@
> > }  #include 
> > }  #include 
> > }  
> > } -#ifndef  int8_t
> > } +#ifndef  _BSD_INT8_T_
> > }  typedef  __int8_tint8_t;
> > } -#define  int8_t  __int8_t
> > } +#define  _BSD_INT8_T_
> > }  #endif
> > 
> >  What's going to define _BSD_INT8_T_ and friends?
> > 
> >  To me, this looks smells like some kind of gross hack to work around
> > broken software.
> > 
> >  [snip]
> > 
> > }-- End of excerpt from co...@sdf.org
> 
> We are. it's a guard to prevent double type definition.

I would argue that the example from

  https://mail-index.netbsd.org/tech-pkg/2018/10/25/msg020395.html

is if not outright wrong then at the very minimum unhygienic, as it
doesn't use the same preprocessor nesting for the definition of the
name and the use of the name, i.e. it incorrectly assumes that the
function will be named "something_uint32_t".

Also your change breaks redefining intN_t types with the preprocessor.
E.g.

#define uint32_t unsigned long long
#include 

is now broken with your change.

-uwe


Re: Definitions of types also as macros

2018-11-06 Thread Kamil Rytarowski
On 06.11.2018 16:07, co...@sdf.org wrote:
> I wanna do this, looks good?
> 

This looks good to me!

> Index: stdint.h
> ===
> RCS file: /cvsroot/src/sys/sys/stdint.h,v
> retrieving revision 1.7
> diff -u -r1.7 stdint.h
> --- stdint.h  22 Apr 2013 21:26:48 -  1.7
> +++ stdint.h  4 Nov 2018 09:35:54 -
> @@ -35,54 +35,54 @@
>  #include 
>  #include 
>  
> -#ifndef  int8_t
> +#ifndef  _BSD_INT8_T_
>  typedef  __int8_tint8_t;
> -#define  int8_t  __int8_t
> +#define  _BSD_INT8_T_
>  #endif
>  
> -#ifndef  uint8_t
> +#ifndef  _BSD_UINT8_T_
>  typedef  __uint8_t   uint8_t;
> -#define  uint8_t __uint8_t
> +#define  _BSD_UINT8_T_
>  #endif
>  
> -#ifndef  int16_t
> +#ifndef  _BSD_INT16_T_
>  typedef  __int16_t   int16_t;
> -#define  int16_t __int16_t
> +#define  _BSD_INT16_T_
>  #endif
>  
> -#ifndef  uint16_t
> +#ifndef  _BSD_UINT16_T_
>  typedef  __uint16_t  uint16_t;
> -#define  uint16_t__uint16_t
> +#define  _BSD_UINT16_T_
>  #endif
>  
> -#ifndef  int32_t
> +#ifndef  _BSD_INT32_T_
>  typedef  __int32_t   int32_t;
> -#define  int32_t __int32_t
> +#define  _BSD_INT32_T_
>  #endif
>  
> -#ifndef  uint32_t
> +#ifndef  _BSD_UINT32_T_
>  typedef  __uint32_t  uint32_t;
> -#define  uint32_t__uint32_t
> +#define  _BSD_UINT32_T_
>  #endif
>  
> -#ifndef  int64_t
> +#ifndef  _BSD_INT64_T_
>  typedef  __int64_t   int64_t;
> -#define  int64_t __int64_t
> +#define  _BSD_INT64_T_
>  #endif
>  
> -#ifndef  uint64_t
> +#ifndef  _BSD_UINT64_T_
>  typedef  __uint64_t  uint64_t;
> -#define  uint64_t__uint64_t
> +#define  _BSD_UINT64_T_
>  #endif
>  
> -#ifndef  intptr_t
> +#ifndef  _BSD_INTPTR_T_
>  typedef  __intptr_t  intptr_t;
> -#define  intptr_t__intptr_t
> +#define  _BSD_INTPTR_T_
>  #endif
>  
> -#ifndef  uintptr_t
> +#ifndef  _BSD_UINTPTR_T_
>  typedef  __uintptr_t uintptr_t;
> -#define  uintptr_t   __uintptr_t
> +#define  _BSD_UINTPTR_T_
>  #endif
>  
>  #include 
> Index: types.h
> ===
> RCS file: /cvsroot/src/sys/sys/types.h,v
> retrieving revision 1.101
> diff -u -r1.101 types.h
> --- types.h   10 Jul 2018 07:40:42 -  1.101
> +++ types.h   4 Nov 2018 09:35:54 -
> @@ -50,44 +50,44 @@
>  
>  #include 
>  
> -#ifndef  int8_t
> +#ifndef  _BSD_INT8_T_
>  typedef  __int8_tint8_t;
> -#define  int8_t  __int8_t
> +#define  _BSD_INT8_T_
>  #endif
>  
> -#ifndef  uint8_t
> +#ifndef  _BSD_UINT8_T_
>  typedef  __uint8_t   uint8_t;
> -#define  uint8_t __uint8_t
> +#define  _BSD_UINT8_T_
>  #endif
>  
> -#ifndef  int16_t
> +#ifndef  _BSD_INT16_T_
>  typedef  __int16_t   int16_t;
> -#define  int16_t __int16_t
> +#define  _BSD_INT16_T_
>  #endif
>  
> -#ifndef  uint16_t
> +#ifndef  _BSD_UINT16_T_
>  typedef  __uint16_t  uint16_t;
> -#define  uint16_t__uint16_t
> +#define  _BSD_UINT16_T_
>  #endif
>  
> -#ifndef  int32_t
> +#ifndef  _BSD_INT32_T_
>  typedef  __int32_t   int32_t;
> -#define  int32_t __int32_t
> +#define  _BSD_INT32_T_
>  #endif
>  
> -#ifndef  uint32_t
> +#ifndef  _BSD_UINT32_T_
>  typedef  __uint32_t  uint32_t;
> -#define  uint32_t__uint32_t
> +#define  _BSD_UINT32_T_
>  #endif
>  
> -#ifndef  int64_t
> +#ifndef  _BSD_INT64_T_
>  typedef  __int64_t   int64_t;
> -#define  int64_t __int64_t
> +#define  _BSD_INT64_T_
>  #endif
>  
> -#ifndef  uint64_t
> +#ifndef  _BSD_UINT64_T_
>  typedef  __uint64_t  uint64_t;
> -#define  uint64_t__uint64_t
> +#define  _BSD_UINT64_T_
>  #endif
>  
>  typedef  uint8_t u_int8_t;
> 




signature.asc
Description: OpenPGP digital signature


Re: Definitions of types also as macros

2018-11-06 Thread coypu
I wanna do this, looks good?

Index: stdint.h
===
RCS file: /cvsroot/src/sys/sys/stdint.h,v
retrieving revision 1.7
diff -u -r1.7 stdint.h
--- stdint.h22 Apr 2013 21:26:48 -  1.7
+++ stdint.h4 Nov 2018 09:35:54 -
@@ -35,54 +35,54 @@
 #include 
 #include 
 
-#ifndefint8_t
+#ifndef_BSD_INT8_T_
 typedef__int8_tint8_t;
-#defineint8_t  __int8_t
+#define_BSD_INT8_T_
 #endif
 
-#ifndefuint8_t
+#ifndef_BSD_UINT8_T_
 typedef__uint8_t   uint8_t;
-#defineuint8_t __uint8_t
+#define_BSD_UINT8_T_
 #endif
 
-#ifndefint16_t
+#ifndef_BSD_INT16_T_
 typedef__int16_t   int16_t;
-#defineint16_t __int16_t
+#define_BSD_INT16_T_
 #endif
 
-#ifndefuint16_t
+#ifndef_BSD_UINT16_T_
 typedef__uint16_t  uint16_t;
-#defineuint16_t__uint16_t
+#define_BSD_UINT16_T_
 #endif
 
-#ifndefint32_t
+#ifndef_BSD_INT32_T_
 typedef__int32_t   int32_t;
-#defineint32_t __int32_t
+#define_BSD_INT32_T_
 #endif
 
-#ifndefuint32_t
+#ifndef_BSD_UINT32_T_
 typedef__uint32_t  uint32_t;
-#defineuint32_t__uint32_t
+#define_BSD_UINT32_T_
 #endif
 
-#ifndefint64_t
+#ifndef_BSD_INT64_T_
 typedef__int64_t   int64_t;
-#defineint64_t __int64_t
+#define_BSD_INT64_T_
 #endif
 
-#ifndefuint64_t
+#ifndef_BSD_UINT64_T_
 typedef__uint64_t  uint64_t;
-#defineuint64_t__uint64_t
+#define_BSD_UINT64_T_
 #endif
 
-#ifndefintptr_t
+#ifndef_BSD_INTPTR_T_
 typedef__intptr_t  intptr_t;
-#defineintptr_t__intptr_t
+#define_BSD_INTPTR_T_
 #endif
 
-#ifndefuintptr_t
+#ifndef_BSD_UINTPTR_T_
 typedef__uintptr_t uintptr_t;
-#defineuintptr_t   __uintptr_t
+#define_BSD_UINTPTR_T_
 #endif
 
 #include 
Index: types.h
===
RCS file: /cvsroot/src/sys/sys/types.h,v
retrieving revision 1.101
diff -u -r1.101 types.h
--- types.h 10 Jul 2018 07:40:42 -  1.101
+++ types.h 4 Nov 2018 09:35:54 -
@@ -50,44 +50,44 @@
 
 #include 
 
-#ifndefint8_t
+#ifndef_BSD_INT8_T_
 typedef__int8_tint8_t;
-#defineint8_t  __int8_t
+#define_BSD_INT8_T_
 #endif
 
-#ifndefuint8_t
+#ifndef_BSD_UINT8_T_
 typedef__uint8_t   uint8_t;
-#defineuint8_t __uint8_t
+#define_BSD_UINT8_T_
 #endif
 
-#ifndefint16_t
+#ifndef_BSD_INT16_T_
 typedef__int16_t   int16_t;
-#defineint16_t __int16_t
+#define_BSD_INT16_T_
 #endif
 
-#ifndefuint16_t
+#ifndef_BSD_UINT16_T_
 typedef__uint16_t  uint16_t;
-#defineuint16_t__uint16_t
+#define_BSD_UINT16_T_
 #endif
 
-#ifndefint32_t
+#ifndef_BSD_INT32_T_
 typedef__int32_t   int32_t;
-#defineint32_t __int32_t
+#define_BSD_INT32_T_
 #endif
 
-#ifndefuint32_t
+#ifndef_BSD_UINT32_T_
 typedef__uint32_t  uint32_t;
-#defineuint32_t__uint32_t
+#define_BSD_UINT32_T_
 #endif
 
-#ifndefint64_t
+#ifndef_BSD_INT64_T_
 typedef__int64_t   int64_t;
-#defineint64_t __int64_t
+#define_BSD_INT64_T_
 #endif
 
-#ifndefuint64_t
+#ifndef_BSD_UINT64_T_
 typedef__uint64_t  uint64_t;
-#defineuint64_t__uint64_t
+#define_BSD_UINT64_T_
 #endif
 
 typedefuint8_t u_int8_t;



Re: Definitions of types also as macros

2018-09-09 Thread Christos Zoulas
On Sep 9,  8:43am, dholland-t...@netbsd.org (David Holland) wrote:
-- Subject: Re: Definitions of types also as macros

| How many headers are these (uint32_t etc. in this case) actually
| supposed to be defined in? The definitions are in both sys/stdint.h
| (which is also stdint.h) and sys/types.h, but sys/types.h includes
| sys/stdint.h under some circumstances and there doesn't seem to be any
| obvious reason it shouldn't just always do that.

The problem is that we have been strict about defining only the types
that are supposed to be defined by the header included (and no others).
If we accept that including the header can define more types, or that
we can have many more files that are just there to define types, then
the problem is solvable.

christos


Re: Definitions of types also as macros

2018-09-09 Thread David Holland
On Sat, Sep 08, 2018 at 11:48:57PM +, Christos Zoulas wrote:
 > The problem is that these are defined in multiple headers and
 > typedef redefinition with the same type is a c11 feature, so the
 > define protects against that. Of course one can use a different
 > macro and this was done before, but this has other disadvantages.

How many headers are these (uint32_t etc. in this case) actually
supposed to be defined in? The definitions are in both sys/stdint.h
(which is also stdint.h) and sys/types.h, but sys/types.h includes
sys/stdint.h under some circumstances and there doesn't seem to be any
obvious reason it shouldn't just always do that.

(Failing that, we could make another header to put the common pieces
in, which is most of sys/stdint.h, or make that header sys/stdint.h
and make the userland stdint.h separate, or various other things.)

 > The other approach is:
 > 
 > #ifdef  _BSD_TIME_T_
 > typedef _BSD_TIME_T_time_t;
 > #undef  _BSD_TIME_T_
 > #endif

I've long felt that the best way to handle this is to give each group
of types that needs to appear in a distinct set of places its own
header file (there aren't actually that many(*)) and rely on the
include guards in the header file to keep from getting multiple copies
of the definitions. It comes out much tidier this way.

This is one of a number of reasons I've long been planning an includes
rototill once we get version control with rename support.


(*) as I recall when I last went through this, when fully sorted out
the only type that actually needed its own header file in this scheme
ended up being size_t, but I'm not sure I'm recalling correctly.

-- 
David A. Holland
dholl...@netbsd.org


Re: Definitions of types also as macros

2018-09-08 Thread Christos Zoulas
In article ,
Christos Zoulas  wrote:
>In article <20180908230813.ga22...@sdf.org>,   wrote:
>>we do this in stdint.h and some other headers:
>>
>>#ifndef uint32_t
>>typedef __uint32_t  uint32_t;
>>#define uint32_t__uint32_t
>>#endif
>>
>>
>>Real-world examples:
>>https://github.com/neovim/neovim/blob/94841e5eaebc3f2fb556056dd676afff21ff5d23/src/nvim/map.h#L12
>>https://github.com/NetBSD/pkgsrc/blob/trunk/www/firefox/patches/patch-servo_components_style_build__gecko.rs
>>
>>And now I ran into:
>>https://cgit.freedesktop.org/mesa/mesa/tree/src/compiler/builtin_type_macros.h#n42
>>
>>Proposal: let's not define the macros?
>>I don't know if there are long running consequences for it.
>
>The problem is that these are defined in multiple headers and
>typedef redefinition with the same type is a c11 feature, so the
>define protects against that. Of course one can use a different
>macro and this was done before, but this has other disadvantages.

The other approach is:

#ifdef  _BSD_TIME_T_
typedef _BSD_TIME_T_time_t;
#undef  _BSD_TIME_T_
#endif

christos



Re: Definitions of types also as macros

2018-09-08 Thread Christos Zoulas
In article <20180908230813.ga22...@sdf.org>,   wrote:
>we do this in stdint.h and some other headers:
>
>#ifndef uint32_t
>typedef __uint32_t  uint32_t;
>#define uint32_t__uint32_t
>#endif
>
>
>Real-world examples:
>https://github.com/neovim/neovim/blob/94841e5eaebc3f2fb556056dd676afff21ff5d23/src/nvim/map.h#L12
>https://github.com/NetBSD/pkgsrc/blob/trunk/www/firefox/patches/patch-servo_components_style_build__gecko.rs
>
>And now I ran into:
>https://cgit.freedesktop.org/mesa/mesa/tree/src/compiler/builtin_type_macros.h#n42
>
>Proposal: let's not define the macros?
>I don't know if there are long running consequences for it.

The problem is that these are defined in multiple headers and
typedef redefinition with the same type is a c11 feature, so the
define protects against that. Of course one can use a different
macro and this was done before, but this has other disadvantages.

christos