Feedback-ID: ic7094478:Fastmail
User-Agent: Cyrus-JMAP/3.9.0-alpha0-172-g9a2dae1853-fm-20230213.001-g9a2dae18
Mime-Version: 1.0
Message-Id: <4eb0228b-53e6-41b2-8daa-d1d07885ca80@app.fastmail.com>
In-Reply-To: <78f08587-7719-40fa-a676-2ba6c4b39b42@t-8ch.de>
References: <20230220202010.37475-1-v@vda.io>
 <20230220202010.37475-3-v@vda.io>
 <78f08587-7719-40fa-a676-2ba6c4b39b42@t-8ch.de>
Date:   Wed, 22 Feb 2023 19:35:17 -0500
From:   "Vincent Dagonneau" <v@vda.io>
To:     =?UTF-8?Q?Thomas_Wei=C3=9Fschuh?= <thomas@t-8ch.de>
Cc:     linux-kernel@vger.kernel.org, "Willy Tarreau" <w@1wt.eu>
Subject: Re: [PATCH v5 2/4] tools/nolibc: add integer types and integer limit macros
Content-Type: text/plain;charset=utf-8
Content-Transfer-Encoding: quoted-printable
Precedence: bulk

Hi Thomas,

On Tue, Feb 21, 2023, at 12:40, Thomas Wei=C3=9Fschuh wrote:
> On Mon, Feb 20, 2023 at 03:20:08PM -0500, Vincent Dagonneau wrote:
>> This commit adds some of the missing integer types to stdint.h and ad=
ds
>> limit macros (e.g. INTN_{MIN,MAX}).
>>=20
>> The reference used for adding these types is
>> https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdint.h.htm=
l.
>>=20
>> We rely on the compiler-defined __LONG_MAX__ to get the right limits =
for
>> ssize_t, size_t, intptr_t, uintptr_t and ptrdiff_t. This compiler
>> constant seem to have been defined at least since GCC 4.1.2 and clang
>> 3.0.0 on x86_64. It is also defined on ARM (32&64), mips and RISC-V.
>>=20
>> Note that the maximum size of size_t is implementation-defined (>6553=
5),
>> in this case I chose to go with unsigned long on all platforms since
>> unsigned long =3D=3D unsigned int on all the platforms we care about.=
 Note
>> that the kernel uses either unsigned int or unsigned long in
>> linux/include/uapi/asm-generic/posix_types.h. These should be equival=
ent
>> for the plaforms we are targeting.
>>=20
>> Also note that the 'fast*' flavor of the types have been chosen to be
>> always 1 byte for '*fast8*' and always long (a.k.a. size_t/ssize_t) f=
or
>> the other variants. I have never seen the 'fast*' types in use in the=
 wild
>> but that seems to be what glibc does.
>>=20
>> Signed-off-by: Vincent Dagonneau <v@vda.io>
>> Signed-off-by: Willy Tarreau <w@1wt.eu>
>> ---
>>  tools/include/nolibc/stdint.h | 77 +++++++++++++++++++++++++++++++++=
++
>>  1 file changed, 77 insertions(+)
>>=20
>> diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/std=
int.h
>> index 4ba264031df9..f7179a583f61 100644
>> --- a/tools/include/nolibc/stdint.h
>> +++ b/tools/include/nolibc/stdint.h
>> @@ -21,4 +21,81 @@ typedef unsigned long     uintptr_t;
>>  typedef   signed long      intptr_t;
>>  typedef   signed long     ptrdiff_t;
>> =20
>> +typedef   int8_t       int_least8_t;
>> +typedef  uint8_t      uint_least8_t;
>> +typedef  int16_t      int_least16_t;
>> +typedef uint16_t     uint_least16_t;
>> +typedef  int32_t      int_least32_t;
>> +typedef uint32_t     uint_least32_t;
>> +typedef  int64_t      int_least64_t;
>> +typedef uint64_t     uint_least64_t;
>> +
>> +typedef   int8_t        int_fast8_t;
>> +typedef  uint8_t       uint_fast8_t;
>> +typedef  ssize_t       int_fast16_t;
>> +typedef   size_t      uint_fast16_t;
>> +typedef  ssize_t       int_fast32_t;
>> +typedef   size_t      uint_fast32_t;
>> +typedef  ssize_t       int_fast64_t;
>> +typedef   size_t      uint_fast64_t;
>> +
>> +typedef  int64_t           intmax_t;
>> +typedef uint64_t          uintmax_t;
>> +
>> +/* limits of integral types */
>> +
>> +#define        INT8_MIN  (-128)
>> +#define       INT16_MIN  (-32767-1)
>> +#define       INT32_MIN  (-2147483647-1)
>> +#define       INT64_MIN  (-9223372036854775807LL-1)
>> +
>> +#define        INT8_MAX  (127)
>> +#define       INT16_MAX  (32767)
>> +#define       INT32_MAX  (2147483647)
>> +#define       INT64_MAX  (9223372036854775807LL)
>> +
>> +#define       UINT8_MAX  (255)
>> +#define      UINT16_MAX  (65535)
>> +#define      UINT32_MAX  (4294967295U)
>> +#define      UINT64_MAX  (18446744073709551615ULL)
>> +
>> +#define  INT_LEAST8_MIN  INT8_MIN
>> +#define INT_LEAST16_MIN  INT16_MIN
>> +#define INT_LEAST32_MIN  INT32_MIN
>> +#define INT_LEAST64_MIN  INT64_MIN
>> +
>> +#define  INT_LEAST8_MAX  INT8_MAX
>> +#define INT_LEAST16_MAX  INT16_MAX
>> +#define INT_LEAST32_MAX  INT32_MAX
>> +#define INT_LEAST64_MAX  INT64_MAX
>> +
>> +#define  UINT_LEAST8_MAX UINT8_MAX
>> +#define UINT_LEAST16_MAX UINT16_MAX
>> +#define UINT_LEAST32_MAX UINT32_MAX
>> +#define UINT_LEAST64_MAX UINT64_MAX
>> +
>> +#define SIZE_MAX         ((size_t)(__LONG_MAX__) * 2 + 1)
>> +#define SSIZE_MIN        (-__LONG_MAX__ - 1)
>
> SSIZE_MIN is not defined by a standard.
> It also doesn't really make sense to have, as ssize_t is only supposed
> to store [-1,SSIZE_MAX].
>
>> +#define SSIZE_MAX        __LONG_MAX__
>
> Apparently SSIZE_MAX can also defined via the compilers <limits.h> as
> used by nolibc-test.c leading to a warning.
> Maybe wrap it in #ifndef SSIZE_MAX.
>
>     In file included from sysroot/x86/include/std.h:21,
> 		     from sysroot/x86/include/stdio.h:12,
> 		     from nolibc-test.c:15:
>     sysroot/x86/include/stdint.h:79: warning: "SSIZE_MAX" redefined
>        79 | #define SSIZE_MAX        __LONG_MAX__
> 	  |=20
>     In file included from /usr/include/limits.h:195,
> 		     from=20
> /usr/lib/gcc/x86_64-pc-linux-gnu/12.2.1/include-fixed/limits.h:203,
> 		     from=20
> /usr/lib/gcc/x86_64-pc-linux-gnu/12.2.1/include-fixed/syslimits.h:7,
> 		     from=20
> /usr/lib/gcc/x86_64-pc-linux-gnu/12.2.1/include-fixed/limits.h:34,
> 		     from nolibc-test.c:6:
>     /usr/include/bits/posix1_lim.h:169: note: this is the location of=20
> the previous definition
>       169 | #  define SSIZE_MAX     LONG_MAX
> 	  |=20
>

Ok, I'll remove the SSIZE_MIN and SSIZE_MAX since neither seem to be in =
the standards page anyway.

Note that ssize_t is still defined in this file. It pre-dates my patch.

>> +#define INTPTR_MIN       SSIZE_MIN
>> +#define INTPTR_MAX       SSIZE_MAX
>> +#define PTRDIFF_MIN      SSIZE_MIN
>> +#define PTRDIFF_MAX      SSIZE_MAX
>> +#define UINTPTR_MAX       SIZE_MAX
>> +
>> +#define  INT_FAST8_MIN  INT8_MIN
>> +#define INT_FAST16_MIN  SSIZE_MIN
>> +#define INT_FAST32_MIN  SSIZE_MIN
>> +#define INT_FAST64_MIN  SSIZE_MIN
>> +
>> +#define  INT_FAST8_MAX  INT8_MAX
>> +#define INT_FAST16_MAX  SSIZE_MAX
>> +#define INT_FAST32_MAX  SSIZE_MAX
>> +#define INT_FAST64_MAX  SSIZE_MAX
>> +
>> +#define  UINT_FAST8_MAX UINT8_MAX
>> +#define UINT_FAST16_MAX SIZE_MAX
>> +#define UINT_FAST32_MAX SIZE_MAX
>> +#define UINT_FAST64_MAX SIZE_MAX
>
> Alignment of values within lines is inconsistent.
>

Thanks, I did a whole lot of alignment in the latest version, hopefully =
it should be ok now.

>> +
>>  #endif /* _NOLIBC_STDINT_H */
>> --=20
>> 2.39.2
>>

Thanks for the review,
Vincent.