Hi Vincent,
I could finally work out the various issues. In short, all of the
problems came from the dependency on __WORDSIZE and the fact that
when not defined, the tests would default to the 32-bit ones. The
reason why it seldom worked was that some cross-compilers configured
to build for the same build and target arch do still mistakenly
include files from /usr/include and inadvertently get some glibc
entries. I changed these locations to rely on __SIZEOF_LONG__ instead
that's provided by the compiler and which I've tested to work fine
since pretty old compilers. For the values I've switched to __LONG_MAX__
that is also defined by the compiler, and this allows us to get rid
of the ifdef and hard-coded values in stdint.h.
Now everything works fine on all supported architectures.
I'd like you to have a look at this patch set, it's yours with
these small changes that I've commented before my s-o-b when
relevant. I would appreciate it if you could recheck everything,
possibly change some stuff if you think it's needed, and also
adjust your commit messages where relevant to match the final
state, dropping my own temporary comments and s-o-b that are
not needed.
One important note, I've based the patch on Paul's latest branch
named "dev.2023.02.06a" here:
https://git.kernel.org/pub/scm/linux/kernel/git/paulmck/linux-rcu.git/
It contains the pending patches that will soon be submitted to Linus,
and support for the s390x platform that I wanted to confirm does work
fine as well. If you need to change some code, please rebase on this
branch before testing.
Thanks!
Willy
--
Vincent Dagonneau (4):
tools/nolibc: add stdint.h
tools/nolibc: add integer types and integer limit macros
tools/nolibc: enlarge column width of tests
tools/nolibc: add tests for the integer limits in stdint.h
tools/include/nolibc/Makefile | 4 +-
tools/include/nolibc/std.h | 15 +-
tools/include/nolibc/stdint.h | 75 ++++++++++
tools/testing/selftests/nolibc/nolibc-test.c | 141 ++++++++++++-------
4 files changed, 170 insertions(+), 65 deletions(-)
create mode 100644 tools/include/nolibc/stdint.h
--
2.35.3
From: Vincent Dagonneau <[email protected]>
This commit adds some of the missing integer types to stdint.h and adds
limit macros (e.g. INTN_{MIN,MAX}).
The reference used for adding these types is
https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdint.h.html.
Note that the maximum size of size_t is implementation-defined (>65535),
in this case I chose to stick with what the kernel uses in
linux/include/uapi/asm-generic/posix_types.h: unsigned int on 32bits and
unsigned long on 64bits.
Signed-off-by: Vincent Dagonneau <[email protected]>
[wt: size_t is always ulong since it matches the word size on all archs
we care for; calculate the SIZE_MAX, INTPTR_MIN and INTPTR_MAX based on
the compiler-provided __LONG_MAX__]
Signed-off-by: Willy Tarreau <[email protected]>
---
tools/include/nolibc/stdint.h | 51 +++++++++++++++++++++++++++++++++++
1 file changed, 51 insertions(+)
diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h
index 4ba264031df9..5f1adf9316ca 100644
--- a/tools/include/nolibc/stdint.h
+++ b/tools/include/nolibc/stdint.h
@@ -21,4 +21,55 @@ typedef unsigned long uintptr_t;
typedef signed long intptr_t;
typedef signed long ptrdiff_t;
+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 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_TYPE__)(__LONG_MAX__) * 2 + 1)
+#define INTPTR_MIN (-__LONG_MAX__ - 1)
+#define INTPTR_MAX __LONG_MAX__
+#define UINTPTR_MAX (SIZE_MAX)
+#define PTRDIFF_MIN INTPTR_MIN
+#define PTRDIFF_MAX INTPTR_MAX
+
#endif /* _NOLIBC_STDINT_H */
--
2.35.3
From: Vincent Dagonneau <[email protected]>
Nolibc works fine for small and limited program however most program
expect integer types to be defined in stdint.h rather than std.h.
This is a quick fix that moves the existing integer definitions in std.h
to stdint.h.
Signed-off-by: Vincent Dagonneau <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
---
tools/include/nolibc/Makefile | 4 ++--
tools/include/nolibc/std.h | 15 +--------------
tools/include/nolibc/stdint.h | 24 ++++++++++++++++++++++++
3 files changed, 27 insertions(+), 16 deletions(-)
create mode 100644 tools/include/nolibc/stdint.h
diff --git a/tools/include/nolibc/Makefile b/tools/include/nolibc/Makefile
index cfd06764b5ae..ec57d3932506 100644
--- a/tools/include/nolibc/Makefile
+++ b/tools/include/nolibc/Makefile
@@ -25,8 +25,8 @@ endif
nolibc_arch := $(patsubst arm64,aarch64,$(ARCH))
arch_file := arch-$(nolibc_arch).h
-all_files := ctype.h errno.h nolibc.h signal.h std.h stdio.h stdlib.h string.h \
- sys.h time.h types.h unistd.h
+all_files := ctype.h errno.h nolibc.h signal.h std.h stdint.h stdio.h stdlib.h \
+ string.h sys.h time.h types.h unistd.h
# install all headers needed to support a bare-metal compiler
all: headers
diff --git a/tools/include/nolibc/std.h b/tools/include/nolibc/std.h
index 1747ae125392..933bc0be7e1c 100644
--- a/tools/include/nolibc/std.h
+++ b/tools/include/nolibc/std.h
@@ -18,20 +18,7 @@
#define NULL ((void *)0)
#endif
-/* stdint types */
-typedef unsigned char uint8_t;
-typedef signed char int8_t;
-typedef unsigned short uint16_t;
-typedef signed short int16_t;
-typedef unsigned int uint32_t;
-typedef signed int int32_t;
-typedef unsigned long long uint64_t;
-typedef signed long long int64_t;
-typedef unsigned long size_t;
-typedef signed long ssize_t;
-typedef unsigned long uintptr_t;
-typedef signed long intptr_t;
-typedef signed long ptrdiff_t;
+#include "stdint.h"
/* those are commonly provided by sys/types.h */
typedef unsigned int dev_t;
diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h
new file mode 100644
index 000000000000..4ba264031df9
--- /dev/null
+++ b/tools/include/nolibc/stdint.h
@@ -0,0 +1,24 @@
+/* SPDX-License-Identifier: LGPL-2.1 OR MIT */
+/*
+ * Standard definitions and types for NOLIBC
+ * Copyright (C) 2023 Vincent Dagonneau <[email protected]>
+ */
+
+#ifndef _NOLIBC_STDINT_H
+#define _NOLIBC_STDINT_H
+
+typedef unsigned char uint8_t;
+typedef signed char int8_t;
+typedef unsigned short uint16_t;
+typedef signed short int16_t;
+typedef unsigned int uint32_t;
+typedef signed int int32_t;
+typedef unsigned long long uint64_t;
+typedef signed long long int64_t;
+typedef unsigned long size_t;
+typedef signed long ssize_t;
+typedef unsigned long uintptr_t;
+typedef signed long intptr_t;
+typedef signed long ptrdiff_t;
+
+#endif /* _NOLIBC_STDINT_H */
--
2.35.3
From: Vincent Dagonneau <[email protected]>
This commit adds tests for the limits added in a previous commit. The
limits are defined in decimal in stdint.h and as hexadecimal in the
tests (e.g. 0x7f = 127 or 0x80 = -128). Hopefully it catches some o the
most egregious mistakes.
Signed-off-by: Vincent Dagonneau <[email protected]>
[wt: rely on the compiler-provided __SIZEOF_LONG__ to decide what values
to expect, and detect its miss to avoid reporting false errors]
Signed-off-by: Willy Tarreau <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 45 +++++++++++++++++++-
1 file changed, 44 insertions(+), 1 deletion(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 882140508d56..290249d6da30 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -561,7 +561,50 @@ int run_syscall(int min, int max)
CASE_TEST(waitpid_child); EXPECT_SYSER(1, waitpid(getpid(), &tmp, WNOHANG), -1, ECHILD); break;
CASE_TEST(write_badf); EXPECT_SYSER(1, write(-1, &tmp, 1), -1, EBADF); break;
CASE_TEST(write_zero); EXPECT_SYSZR(1, write(1, &tmp, 0)); break;
- case __LINE__:
+ CASE_TEST(limit_int8_max); EXPECT_EQ(1, INT8_MAX, (int8_t) 0x7f); break;
+ CASE_TEST(limit_int8_min); EXPECT_EQ(1, INT8_MIN, (int8_t) 0x80); break;
+ CASE_TEST(limit_uint8_max); EXPECT_EQ(1, UINT8_MAX, (uint8_t) 0xff); break;
+ CASE_TEST(limit_int16_max); EXPECT_EQ(1, INT16_MAX, (int16_t) 0x7fff); break;
+ CASE_TEST(limit_int16_min); EXPECT_EQ(1, INT16_MIN, (int16_t) 0x8000); break;
+ CASE_TEST(limit_uint16_max); EXPECT_EQ(1, UINT16_MAX, (uint16_t) 0xffff); break;
+ CASE_TEST(limit_int32_max); EXPECT_EQ(1, INT32_MAX, (int32_t) 0x7fffffff); break;
+ CASE_TEST(limit_int32_min); EXPECT_EQ(1, INT32_MIN, (int32_t) 0x80000000); break;
+ CASE_TEST(limit_uint32_max); EXPECT_EQ(1, UINT32_MAX, (uint32_t) 0xffffffff); break;
+ CASE_TEST(limit_int64_max); EXPECT_EQ(1, INT64_MAX, (int64_t) 0x7fffffffffffffff); break;
+ CASE_TEST(limit_int64_min); EXPECT_EQ(1, INT64_MIN, (int64_t) 0x8000000000000000); break;
+ CASE_TEST(limit_uint64_max); EXPECT_EQ(1, UINT64_MAX, (uint64_t) 0xffffffffffffffff); break;
+ CASE_TEST(limit_int_least8_max); EXPECT_EQ(1, INT_LEAST8_MAX, (int_least8_t) 0x7f); break;
+ CASE_TEST(limit_int_least8_min); EXPECT_EQ(1, INT_LEAST8_MIN, (int_least8_t) 0x80); break;
+ CASE_TEST(limit_uint_least8_max); EXPECT_EQ(1, UINT_LEAST8_MAX, (uint_least8_t) 0xff); break;
+ CASE_TEST(limit_int_least16_max); EXPECT_EQ(1, INT_LEAST16_MAX, (int_least16_t) 0x7fff); break;
+ CASE_TEST(limit_int_least16_min); EXPECT_EQ(1, INT_LEAST16_MIN, (int_least16_t) 0x8000); break;
+ CASE_TEST(limit_uint_least16_max); EXPECT_EQ(1, UINT_LEAST16_MAX, (uint_least16_t) 0xffff); break;
+ CASE_TEST(limit_int_least32_max); EXPECT_EQ(1, INT_LEAST32_MAX, (int_least32_t) 0x7fffffff); break;
+ CASE_TEST(limit_int_least32_min); EXPECT_EQ(1, INT_LEAST32_MIN, (int_least32_t) 0x80000000); break;
+ CASE_TEST(limit_uint_least32_max); EXPECT_EQ(1, UINT_LEAST32_MAX, (uint_least32_t) 0xffffffffU); break;
+#if __SIZEOF_LONG__ == 8
+ CASE_TEST(limit_int_least64_max); EXPECT_EQ(1, INT_LEAST64_MAX, (int_least64_t) 0x7fffffffffffffffLL); break;
+ CASE_TEST(limit_int_least64_min); EXPECT_EQ(1, INT_LEAST64_MIN, (int_least64_t) 0x8000000000000000LL); break;
+ CASE_TEST(limit_uint_least64_max); EXPECT_EQ(1, UINT_LEAST64_MAX, (uint_least64_t) 0xffffffffffffffffULL); break;
+ CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (intptr_t) 0x8000000000000000LL); break;
+ CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (intptr_t) 0x7fffffffffffffffLL); break;
+ CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (uintptr_t) 0xffffffffffffffffULL); break;
+ CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x8000000000000000LL); break;
+ CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffffffffffffLL); break;
+ CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffffffffffULL); break;
+#elif __SIZEOF_LONG__ == 4
+ CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (intptr_t) 0x80000000); break;
+ CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (intptr_t) 0x7fffffff); break;
+ CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (uintptr_t) 0xffffffffU); break;
+ CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break;
+ CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break;
+ CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break;
+ CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break;
+ CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffU); break;
+#else
+# warning "__SIZEOF_LONG__ is undefined"
+#endif /* __WORDSIZE == 64 */
+ case __LINE__:
return ret; /* must be last */
/* note: do not set any defaults so as to permit holes above */
}
--
2.35.3
From: Vincent Dagonneau <[email protected]>
This commit enlarges the column width from 40 to 64 characters to make
room for longer tests
Signed-off-by: Vincent Dagonneau <[email protected]>
Signed-off-by: Willy Tarreau <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 96 ++++++++++----------
1 file changed, 48 insertions(+), 48 deletions(-)
diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index c4a0c915139c..882140508d56 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -130,111 +130,111 @@ static int pad_spc(int llen, int cnt, const char *fmt, ...)
*/
#define EXPECT_ZR(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_zr(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_zr(expr, llen); } while (0)
static int expect_zr(int expr, int llen)
{
int ret = !(expr == 0);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_NZ(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_nz(expr, llen; } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_nz(expr, llen; } while (0)
static int expect_nz(int expr, int llen)
{
int ret = !(expr != 0);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_EQ(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_eq(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_eq(expr, llen, val); } while (0)
-static int expect_eq(int expr, int llen, int val)
+static int expect_eq(uint64_t expr, int llen, uint64_t val)
{
int ret = !(expr == val);
- llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ llen += printf(" = %lld ", expr);
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_NE(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_ne(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_ne(expr, llen, val); } while (0)
static int expect_ne(int expr, int llen, int val)
{
int ret = !(expr != val);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_GE(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_ge(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_ge(expr, llen, val); } while (0)
static int expect_ge(int expr, int llen, int val)
{
int ret = !(expr >= val);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_GT(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_gt(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_gt(expr, llen, val); } while (0)
static int expect_gt(int expr, int llen, int val)
{
int ret = !(expr > val);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_LE(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_le(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_le(expr, llen, val); } while (0)
static int expect_le(int expr, int llen, int val)
{
int ret = !(expr <= val);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_LT(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_lt(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_lt(expr, llen, val); } while (0)
static int expect_lt(int expr, int llen, int val)
{
int ret = !(expr < val);
llen += printf(" = %d ", expr);
- pad_spc(llen, 40, ret ? "[FAIL]\n" : " [OK]\n");
+ pad_spc(llen, 64, ret ? "[FAIL]\n" : " [OK]\n");
return ret;
}
#define EXPECT_SYSZR(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_syszr(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_syszr(expr, llen); } while (0)
static int expect_syszr(int expr, int llen)
{
@@ -243,17 +243,17 @@ static int expect_syszr(int expr, int llen)
if (expr) {
ret = 1;
llen += printf(" = %d %s ", expr, errorname(errno));
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
llen += printf(" = %d ", expr);
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_SYSEQ(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_syseq(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_syseq(expr, llen, val); } while (0)
static int expect_syseq(int expr, int llen, int val)
{
@@ -262,17 +262,17 @@ static int expect_syseq(int expr, int llen, int val)
if (expr != val) {
ret = 1;
llen += printf(" = %d %s ", expr, errorname(errno));
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
llen += printf(" = %d ", expr);
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_SYSNE(cond, expr, val) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_sysne(expr, llen, val); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_sysne(expr, llen, val); } while (0)
static int expect_sysne(int expr, int llen, int val)
{
@@ -281,17 +281,17 @@ static int expect_sysne(int expr, int llen, int val)
if (expr == val) {
ret = 1;
llen += printf(" = %d %s ", expr, errorname(errno));
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
llen += printf(" = %d ", expr);
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_SYSER(cond, expr, expret, experr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_syserr(expr, expret, experr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_syserr(expr, expret, experr, llen); } while (0)
static int expect_syserr(int expr, int expret, int experr, int llen)
{
@@ -302,16 +302,16 @@ static int expect_syserr(int expr, int expret, int experr, int llen)
if (expr != expret || _errno != experr) {
ret = 1;
llen += printf(" != (%d %s) ", expret, errorname(experr));
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_PTRZR(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_ptrzr(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_ptrzr(expr, llen); } while (0)
static int expect_ptrzr(const void *expr, int llen)
{
@@ -320,16 +320,16 @@ static int expect_ptrzr(const void *expr, int llen)
llen += printf(" = <%p> ", expr);
if (expr) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_PTRNZ(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_ptrnz(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_ptrnz(expr, llen); } while (0)
static int expect_ptrnz(const void *expr, int llen)
{
@@ -338,16 +338,16 @@ static int expect_ptrnz(const void *expr, int llen)
llen += printf(" = <%p> ", expr);
if (!expr) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_STRZR(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_strzr(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_strzr(expr, llen); } while (0)
static int expect_strzr(const char *expr, int llen)
{
@@ -356,16 +356,16 @@ static int expect_strzr(const char *expr, int llen)
llen += printf(" = <%s> ", expr);
if (expr) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_STRNZ(cond, expr) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_strnz(expr, llen); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_strnz(expr, llen); } while (0)
static int expect_strnz(const char *expr, int llen)
{
@@ -374,16 +374,16 @@ static int expect_strnz(const char *expr, int llen)
llen += printf(" = <%s> ", expr);
if (!expr) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_STREQ(cond, expr, cmp) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_streq(expr, llen, cmp); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_streq(expr, llen, cmp); } while (0)
static int expect_streq(const char *expr, int llen, const char *cmp)
{
@@ -392,16 +392,16 @@ static int expect_streq(const char *expr, int llen, const char *cmp)
llen += printf(" = <%s> ", expr);
if (strcmp(expr, cmp) != 0) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
#define EXPECT_STRNE(cond, expr, cmp) \
- do { if (!cond) pad_spc(llen, 40, "[SKIPPED]\n"); else ret += expect_strne(expr, llen, cmp); } while (0)
+ do { if (!cond) pad_spc(llen, 64, "[SKIPPED]\n"); else ret += expect_strne(expr, llen, cmp); } while (0)
static int expect_strne(const char *expr, int llen, const char *cmp)
{
@@ -410,9 +410,9 @@ static int expect_strne(const char *expr, int llen, const char *cmp)
llen += printf(" = <%s> ", expr);
if (strcmp(expr, cmp) == 0) {
ret = 1;
- llen += pad_spc(llen, 40, "[FAIL]\n");
+ llen += pad_spc(llen, 64, "[FAIL]\n");
} else {
- llen += pad_spc(llen, 40, " [OK]\n");
+ llen += pad_spc(llen, 64, " [OK]\n");
}
return ret;
}
--
2.35.3
On Sun, Feb 19, 2023 at 07:51:33PM +0100, Willy Tarreau wrote:
> From: Vincent Dagonneau <[email protected]>
>
> This commit adds tests for the limits added in a previous commit. The
> limits are defined in decimal in stdint.h and as hexadecimal in the
> tests (e.g. 0x7f = 127 or 0x80 = -128). Hopefully it catches some o the
> most egregious mistakes.
>
> Signed-off-by: Vincent Dagonneau <[email protected]>
> [wt: rely on the compiler-provided __SIZEOF_LONG__ to decide what values
> to expect, and detect its miss to avoid reporting false errors]
> Signed-off-by: Willy Tarreau <[email protected]>
> ---
> tools/testing/selftests/nolibc/nolibc-test.c | 45 +++++++++++++++++++-
> 1 file changed, 44 insertions(+), 1 deletion(-)
>
> diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
> index 882140508d56..290249d6da30 100644
> --- a/tools/testing/selftests/nolibc/nolibc-test.c
> +++ b/tools/testing/selftests/nolibc/nolibc-test.c
> @@ -561,7 +561,50 @@ int run_syscall(int min, int max)
> CASE_TEST(waitpid_child); EXPECT_SYSER(1, waitpid(getpid(), &tmp, WNOHANG), -1, ECHILD); break;
> CASE_TEST(write_badf); EXPECT_SYSER(1, write(-1, &tmp, 1), -1, EBADF); break;
> CASE_TEST(write_zero); EXPECT_SYSZR(1, write(1, &tmp, 0)); break;
> - case __LINE__:
> + CASE_TEST(limit_int8_max); EXPECT_EQ(1, INT8_MAX, (int8_t) 0x7f); break;
> + CASE_TEST(limit_int8_min); EXPECT_EQ(1, INT8_MIN, (int8_t) 0x80); break;
> + CASE_TEST(limit_uint8_max); EXPECT_EQ(1, UINT8_MAX, (uint8_t) 0xff); break;
> + CASE_TEST(limit_int16_max); EXPECT_EQ(1, INT16_MAX, (int16_t) 0x7fff); break;
> + CASE_TEST(limit_int16_min); EXPECT_EQ(1, INT16_MIN, (int16_t) 0x8000); break;
> + CASE_TEST(limit_uint16_max); EXPECT_EQ(1, UINT16_MAX, (uint16_t) 0xffff); break;
> + CASE_TEST(limit_int32_max); EXPECT_EQ(1, INT32_MAX, (int32_t) 0x7fffffff); break;
> + CASE_TEST(limit_int32_min); EXPECT_EQ(1, INT32_MIN, (int32_t) 0x80000000); break;
> + CASE_TEST(limit_uint32_max); EXPECT_EQ(1, UINT32_MAX, (uint32_t) 0xffffffff); break;
> + CASE_TEST(limit_int64_max); EXPECT_EQ(1, INT64_MAX, (int64_t) 0x7fffffffffffffff); break;
> + CASE_TEST(limit_int64_min); EXPECT_EQ(1, INT64_MIN, (int64_t) 0x8000000000000000); break;
> + CASE_TEST(limit_uint64_max); EXPECT_EQ(1, UINT64_MAX, (uint64_t) 0xffffffffffffffff); break;
> + CASE_TEST(limit_int_least8_max); EXPECT_EQ(1, INT_LEAST8_MAX, (int_least8_t) 0x7f); break;
> + CASE_TEST(limit_int_least8_min); EXPECT_EQ(1, INT_LEAST8_MIN, (int_least8_t) 0x80); break;
> + CASE_TEST(limit_uint_least8_max); EXPECT_EQ(1, UINT_LEAST8_MAX, (uint_least8_t) 0xff); break;
> + CASE_TEST(limit_int_least16_max); EXPECT_EQ(1, INT_LEAST16_MAX, (int_least16_t) 0x7fff); break;
> + CASE_TEST(limit_int_least16_min); EXPECT_EQ(1, INT_LEAST16_MIN, (int_least16_t) 0x8000); break;
> + CASE_TEST(limit_uint_least16_max); EXPECT_EQ(1, UINT_LEAST16_MAX, (uint_least16_t) 0xffff); break;
> + CASE_TEST(limit_int_least32_max); EXPECT_EQ(1, INT_LEAST32_MAX, (int_least32_t) 0x7fffffff); break;
> + CASE_TEST(limit_int_least32_min); EXPECT_EQ(1, INT_LEAST32_MIN, (int_least32_t) 0x80000000); break;
> + CASE_TEST(limit_uint_least32_max); EXPECT_EQ(1, UINT_LEAST32_MAX, (uint_least32_t) 0xffffffffU); break;
> +#if __SIZEOF_LONG__ == 8
> + CASE_TEST(limit_int_least64_max); EXPECT_EQ(1, INT_LEAST64_MAX, (int_least64_t) 0x7fffffffffffffffLL); break;
> + CASE_TEST(limit_int_least64_min); EXPECT_EQ(1, INT_LEAST64_MIN, (int_least64_t) 0x8000000000000000LL); break;
> + CASE_TEST(limit_uint_least64_max); EXPECT_EQ(1, UINT_LEAST64_MAX, (uint_least64_t) 0xffffffffffffffffULL); break;
int64 types are also defined for 32bit platforms, but the tests are only
ran for 64bits. Is this intentional?
> + CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (intptr_t) 0x8000000000000000LL); break;
> + CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (intptr_t) 0x7fffffffffffffffLL); break;
> + CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (uintptr_t) 0xffffffffffffffffULL); break;
> + CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x8000000000000000LL); break;
> + CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffffffffffffLL); break;
> + CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffffffffffULL); break;
> +#elif __SIZEOF_LONG__ == 4
> + CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (intptr_t) 0x80000000); break;
> + CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (intptr_t) 0x7fffffff); break;
> + CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (uintptr_t) 0xffffffffU); break;
> + CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break;
> + CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break;
> + CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break;
> + CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break;
ptrdiff tests are duplicate.
> + CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffU); break;
> +#else
> +# warning "__SIZEOF_LONG__ is undefined"
Why not #error?
> +#endif /* __WORDSIZE == 64 */
#endif comment is now incorrect
> + case __LINE__:
The "case" should be further left, no?
> return ret; /* must be last */
> /* note: do not set any defaults so as to permit holes above */
> }
> --
> 2.35.3
>
Hi Thomas,
On Sun, Feb 19, 2023 at 07:04:04PM +0000, Thomas Wei?schuh wrote:
> > +#elif __SIZEOF_LONG__ == 4
> > + CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (intptr_t) 0x80000000); break;
> > + CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (intptr_t) 0x7fffffff); break;
> > + CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (uintptr_t) 0xffffffffU); break;
> > + CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break;
> > + CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break;
> > + CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break;
> > + CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break;
>
> ptrdiff tests are duplicate.
Argh, I thought I had already removed these duplicates, I noticed them
previously indeed. Vincent, please address this in your next iteration.
> > + CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffU); break;
> > +#else
> > +# warning "__SIZEOF_LONG__ is undefined"
>
> Why not #error?
It's just a matter of choice. Since the tool's goal is to spot errors,
and if possible several at once, I find it preferable to still not fail
on other tests, as often when you get multiple failures it's easier to
figure what's going on. During the last test session I precisely had a
build error that was quite annoying because once I managed to fix it I
figured the fix was not the right one regarding other places.
Alternately we could probably just add one line that always reports a
failure like the other ones (it would be even better so that we can
compare all outputs and still know that something fails):
+#else
+ CASE_TEST(__SIZEOF_LONG__defined); EXPECT_EQ(1, 1, 0); break;
> > +#endif /* __WORDSIZE == 64 */
>
> #endif comment is now incorrect
Good catch indeed!
>
> > + case __LINE__:
>
> The "case" should be further left, no?
You're right!
Thank you!
Willy
On Sun, Feb 19, 2023 at 07:51:31PM +0100, Willy Tarreau wrote:
> From: Vincent Dagonneau <[email protected]>
>
> This commit adds some of the missing integer types to stdint.h and adds
> limit macros (e.g. INTN_{MIN,MAX}).
>
> The reference used for adding these types is
> https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdint.h.html.
>
> Note that the maximum size of size_t is implementation-defined (>65535),
> in this case I chose to stick with what the kernel uses in
> linux/include/uapi/asm-generic/posix_types.h: unsigned int on 32bits and
> unsigned long on 64bits.
>
> Signed-off-by: Vincent Dagonneau <[email protected]>
> [wt: size_t is always ulong since it matches the word size on all archs
> we care for; calculate the SIZE_MAX, INTPTR_MIN and INTPTR_MAX based on
> the compiler-provided __LONG_MAX__]
> Signed-off-by: Willy Tarreau <[email protected]>
> ---
> tools/include/nolibc/stdint.h | 51 +++++++++++++++++++++++++++++++++++
> 1 file changed, 51 insertions(+)
>
> diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h
> index 4ba264031df9..5f1adf9316ca 100644
> --- a/tools/include/nolibc/stdint.h
> +++ b/tools/include/nolibc/stdint.h
> @@ -21,4 +21,55 @@ typedef unsigned long uintptr_t;
> typedef signed long intptr_t;
> typedef signed long ptrdiff_t;
>
> +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 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_TYPE__)(__LONG_MAX__) * 2 + 1)
Nit: size_t would look nicer than __SIZE_TYPE__.
Also SSIZE_MAX could be added.
> +#define INTPTR_MIN (-__LONG_MAX__ - 1)
> +#define INTPTR_MAX __LONG_MAX__
> +#define UINTPTR_MAX (SIZE_MAX)
Braces seem unnecessary.
> +#define PTRDIFF_MIN INTPTR_MIN
> +#define PTRDIFF_MAX INTPTR_MAX
> +
> #endif /* _NOLIBC_STDINT_H */
> --
> 2.35.3
>
On Sun, Feb 19, 2023 at 07:16:57PM +0000, Thomas Wei?schuh wrote:
> On Sun, Feb 19, 2023 at 07:51:31PM +0100, Willy Tarreau wrote:
> > From: Vincent Dagonneau <[email protected]>
> >
> > This commit adds some of the missing integer types to stdint.h and adds
> > limit macros (e.g. INTN_{MIN,MAX}).
> >
> > The reference used for adding these types is
> > https://pubs.opengroup.org/onlinepubs/009695399/basedefs/stdint.h.html.
> >
> > Note that the maximum size of size_t is implementation-defined (>65535),
> > in this case I chose to stick with what the kernel uses in
> > linux/include/uapi/asm-generic/posix_types.h: unsigned int on 32bits and
> > unsigned long on 64bits.
> >
> > Signed-off-by: Vincent Dagonneau <[email protected]>
> > [wt: size_t is always ulong since it matches the word size on all archs
> > we care for; calculate the SIZE_MAX, INTPTR_MIN and INTPTR_MAX based on
> > the compiler-provided __LONG_MAX__]
> > Signed-off-by: Willy Tarreau <[email protected]>
> > ---
> > tools/include/nolibc/stdint.h | 51 +++++++++++++++++++++++++++++++++++
> > 1 file changed, 51 insertions(+)
> >
> > diff --git a/tools/include/nolibc/stdint.h b/tools/include/nolibc/stdint.h
> > index 4ba264031df9..5f1adf9316ca 100644
> > --- a/tools/include/nolibc/stdint.h
> > +++ b/tools/include/nolibc/stdint.h
> > @@ -21,4 +21,55 @@ typedef unsigned long uintptr_t;
> > typedef signed long intptr_t;
> > typedef signed long ptrdiff_t;
> >
> > +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 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_TYPE__)(__LONG_MAX__) * 2 + 1)
>
> Nit: size_t would look nicer than __SIZE_TYPE__.
Thanks, I hesitated.
> Also SSIZE_MAX could be added.
Ah I hadn't noticed that ssize_t was defined in the file, in this case
I agree it would be better to have it as well.
> > +#define INTPTR_MIN (-__LONG_MAX__ - 1)
> > +#define INTPTR_MAX __LONG_MAX__
> > +#define UINTPTR_MAX (SIZE_MAX)
>
> Braces seem unnecessary.
Indeed.
Thanks!
Willy
From: Willy Tarreau
> Sent: 19 February 2023 18:52
>
> This commit adds some of the missing integer types to stdint.h and adds
> limit macros (e.g. INTN_{MIN,MAX}).
>
...
>
> +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;
The are also the 'fast' variants.
Although I'd be tempted to either not define the 'least'
or 'fast' types (or maybe define them all as 'long').
The only code I've ever seen that used uint_fast32_t
got 'confused' when it was 64 bits.
...
> +/* limits of integral types */
> +
> +#define INT8_MIN (-128)
> +#define INT16_MIN (-32767-1)
> +#define INT32_MIN (-2147483647-1)
> +#define INT64_MIN (-9223372036854775807LL-1)
Those big decimal numbers are difficult to check!
A typo would be unfortunate!
Maybe (eg):
#define INT64_MIN (-INT64_MAX - 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)
None of those need brackets.
Defining in hex would be more readable.
Although all the 'f' get hard to count as well.
Given that the types are defined in the same file, why
not use ~0u and ~0ull for UINT32_MAX and UINT64_MAX.
Should UINT8_MAX and UINT16_MAX be unsigned constants?
(Or even be cast to the corresponding type?)
It doesn't affect arithmetic, but would make a difference
to the over-zealous type checking in the kernel min/max
defines.
David
-
Registered Address Lakeside, Bramley Road, Mount Farm, Milton Keynes, MK1 1PT, UK
Registration No: 1397386 (Wales)
Hi David,
On Mon, Feb 20, 2023 at 09:14:04AM +0000, David Laight wrote:
> From: Willy Tarreau
> > Sent: 19 February 2023 18:52
> >
> > This commit adds some of the missing integer types to stdint.h and adds
> > limit macros (e.g. INTN_{MIN,MAX}).
> >
> ...
> >
> > +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;
>
> The are also the 'fast' variants.
> Although I'd be tempted to either not define the 'least'
> or 'fast' types (or maybe define them all as 'long').
> The only code I've ever seen that used uint_fast32_t
> got 'confused' when it was 64 bits.
Honestly I've never seen either the "least" nor the "fast" variants
used and am not at all convinced we need them. But they're not causing
issues either and I'm fine with Vincent adding them.
> ...
> > +/* limits of integral types */
> > +
> > +#define INT8_MIN (-128)
> > +#define INT16_MIN (-32767-1)
> > +#define INT32_MIN (-2147483647-1)
> > +#define INT64_MIN (-9223372036854775807LL-1)
>
> Those big decimal numbers are difficult to check!
> A typo would be unfortunate!
That's also the purpose of the test!
> Maybe (eg):
> #define INT64_MIN (-INT64_MAX - 1)
Some would argue that it's less easy to check when you're grepping for
a value. How often have you found yourself bouncing between glibc
include files looking for a definition for example ? I'm not sold on
either choice, it's indeed just a matter of taste in the end.
> > +#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)
>
> None of those need brackets.
Most likely it was done to be more uniform with the rest above.
> Defining in hex would be more readable.
Sure they would but it's not the same. Hex numbers are usually
considered as neither positive nor negative probably because they're
more commonly used to manipulate bits rather than quantities, and often
they will not trigger warnings on overflows. Look for example:
$ cat yy.c
int a = 0x80000000;
int b = -0x80000000;
int c = 2147483648;
int d = -2147483648;
int e = 0x80000000 + 1;
int f = 0x80000000 - 1;
int g = 2147483648 + 1;
int h = -2147483648 - 1;
$ clang -W -Wall -Wextra -c yy.c
yy.c:3:9: warning: implicit conversion from 'long' to 'int' changes value from 2147483648 to -2147483648 [-Wconstant-conversion]
int c = 2147483648;
~ ^~~~~~~~~~
yy.c:8:21: warning: implicit conversion from 'long' to 'int' changes value from 2147483649 to -2147483647 [-Wconstant-conversion]
int g = 2147483648 + 1;
~ ~~~~~~~~~~~^~~
yy.c:9:21: warning: implicit conversion from 'long' to 'int' changes value from -2147483649 to 2147483647 [-Wconstant-conversion]
int h = -2147483648 - 1;
~ ~~~~~~~~~~~~^~~
Notice how the hex ones didn't complain. Just for this I would
rather keep the decimal ones, even if less readable.
> Although all the 'f' get hard to count as well.
> Given that the types are defined in the same file, why
> not use ~0u and ~0ull for UINT32_MAX and UINT64_MAX.
That's what I usually do but here I think it's mostly to stay
consistent across the whole file.
> Should UINT8_MAX and UINT16_MAX be unsigned constants?
> (Or even be cast to the corresponding type?)
Same, better not if we want to keep the compiler's warnings in case
of wrong assignment. Just compare the outputs of:
char c = UINT8_MAX;
when UINT8_MAX is defined as 255 and 255U. Only the former gives me:
yy.c:11:11: warning: implicit conversion from 'int' to 'char' changes value from 255 to -1 [-Wconstant-conversion]
char cc = 255;
~~ ^~~
Thus it gives one extra opportunity to spot a typo.
Thanks!
Willy
Hi David,
On Mon, Feb 20, 2023, at 09:47, Willy Tarreau wrote:
> Hi David,
>
> On Mon, Feb 20, 2023 at 09:14:04AM +0000, David Laight wrote:
>> From: Willy Tarreau
>> > Sent: 19 February 2023 18:52
>> >
>> > This commit adds some of the missing integer types to stdint.h and adds
>> > limit macros (e.g. INTN_{MIN,MAX}).
>> >
>> ...
>> >
>> > +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;
>>
>> The are also the 'fast' variants.
>> Although I'd be tempted to either not define the 'least'
>> or 'fast' types (or maybe define them all as 'long').
>> The only code I've ever seen that used uint_fast32_t
>> got 'confused' when it was 64 bits.
>
> Honestly I've never seen either the "least" nor the "fast" variants
> used and am not at all convinced we need them. But they're not causing
> issues either and I'm fine with Vincent adding them.
>
I have never seen them used in the wild either but I included them in the v5.
>> ...
>> > +/* limits of integral types */
>> > +
>> > +#define INT8_MIN (-128)
>> > +#define INT16_MIN (-32767-1)
>> > +#define INT32_MIN (-2147483647-1)
>> > +#define INT64_MIN (-9223372036854775807LL-1)
>>
>> Those big decimal numbers are difficult to check!
>> A typo would be unfortunate!
>
> That's also the purpose of the test!
>
My rationale for writing the full decimal in the header file is that we have a check for that in the tests. Furthermore, I wrote the number in decimal there but in hexadecimal in the test. Hopefully, at least one of them is right and catches any mistake.
>> Maybe (eg):
>> #define INT64_MIN (-INT64_MAX - 1)
>
> Some would argue that it's less easy to check when you're grepping for
> a value. How often have you found yourself bouncing between glibc
> include files looking for a definition for example ? I'm not sold on
> either choice, it's indeed just a matter of taste in the end.
>
>> > +#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)
>>
>> None of those need brackets.
>
> Most likely it was done to be more uniform with the rest above.
>
It is mostly comestic, yes.
>> Defining in hex would be more readable.
>
> Sure they would but it's not the same. Hex numbers are usually
> considered as neither positive nor negative probably because they're
> more commonly used to manipulate bits rather than quantities, and often
> they will not trigger warnings on overflows. Look for example:
>
> $ cat yy.c
> int a = 0x80000000;
> int b = -0x80000000;
> int c = 2147483648;
> int d = -2147483648;
>
> int e = 0x80000000 + 1;
> int f = 0x80000000 - 1;
> int g = 2147483648 + 1;
> int h = -2147483648 - 1;
>
> $ clang -W -Wall -Wextra -c yy.c
> yy.c:3:9: warning: implicit conversion from 'long' to 'int' changes
> value from 2147483648 to -2147483648 [-Wconstant-conversion]
> int c = 2147483648;
> ~ ^~~~~~~~~~
> yy.c:8:21: warning: implicit conversion from 'long' to 'int' changes
> value from 2147483649 to -2147483647 [-Wconstant-conversion]
> int g = 2147483648 + 1;
> ~ ~~~~~~~~~~~^~~
> yy.c:9:21: warning: implicit conversion from 'long' to 'int' changes
> value from -2147483649 to 2147483647 [-Wconstant-conversion]
> int h = -2147483648 - 1;
> ~ ~~~~~~~~~~~~^~~
>
> Notice how the hex ones didn't complain. Just for this I would
> rather keep the decimal ones, even if less readable.
>
Additionally, like previously mentioned, the tests are based on the hex representation as it is indeed easier to read.
>> Although all the 'f' get hard to count as well.
>> Given that the types are defined in the same file, why
>> not use ~0u and ~0ull for UINT32_MAX and UINT64_MAX.
>
> That's what I usually do but here I think it's mostly to stay
> consistent across the whole file.
>
Indeed, it is also mostly cosmetic.
>> Should UINT8_MAX and UINT16_MAX be unsigned constants?
>> (Or even be cast to the corresponding type?)
>
> Same, better not if we want to keep the compiler's warnings in case
> of wrong assignment. Just compare the outputs of:
>
> char c = UINT8_MAX;
>
> when UINT8_MAX is defined as 255 and 255U. Only the former gives me:
>
> yy.c:11:11: warning: implicit conversion from 'int' to 'char' changes
> value from 255 to -1 [-Wconstant-conversion]
> char cc = 255;
> ~~ ^~~
>
> Thus it gives one extra opportunity to spot a typo.
>
> Thanks!
> Willy
Thank you for the review!
Hi Thomas,
On Sun, Feb 19, 2023, at 14:15, Willy Tarreau wrote:
> Hi Thomas,
>
> On Sun, Feb 19, 2023 at 07:04:04PM +0000, Thomas Weißschuh wrote:
>> > +#elif __SIZEOF_LONG__ == 4
>> > + CASE_TEST(limit_intptr_min); EXPECT_EQ(1, INTPTR_MIN, (intptr_t) 0x80000000); break;
>> > + CASE_TEST(limit_intptr_max); EXPECT_EQ(1, INTPTR_MAX, (intptr_t) 0x7fffffff); break;
>> > + CASE_TEST(limit_uintptr_max); EXPECT_EQ(1, UINTPTR_MAX, (uintptr_t) 0xffffffffU); break;
>> > + CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break;
>> > + CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break;
>> > + CASE_TEST(limit_ptrdiff_min); EXPECT_EQ(1, PTRDIFF_MIN, (ptrdiff_t) 0x80000000); break;
>> > + CASE_TEST(limit_ptrdiff_max); EXPECT_EQ(1, PTRDIFF_MAX, (ptrdiff_t) 0x7fffffff); break;
>>
>> ptrdiff tests are duplicate.
>
> Argh, I thought I had already removed these duplicates, I noticed them
> previously indeed. Vincent, please address this in your next iteration.
>
Oops, my mistake, sorry about that. I removed it from v5.
>> > + CASE_TEST(limit_size_max); EXPECT_EQ(1, SIZE_MAX, (size_t) 0xffffffffU); break;
>> > +#else
>> > +# warning "__SIZEOF_LONG__ is undefined"
>>
>> Why not #error?
>
> It's just a matter of choice. Since the tool's goal is to spot errors,
> and if possible several at once, I find it preferable to still not fail
> on other tests, as often when you get multiple failures it's easier to
> figure what's going on. During the last test session I precisely had a
> build error that was quite annoying because once I managed to fix it I
> figured the fix was not the right one regarding other places.
>
> Alternately we could probably just add one line that always reports a
> failure like the other ones (it would be even better so that we can
> compare all outputs and still know that something fails):
>
> +#else
> + CASE_TEST(__SIZEOF_LONG__defined); EXPECT_EQ(1, 1, 0); break;
>
>> > +#endif /* __WORDSIZE == 64 */
>>
>> #endif comment is now incorrect
>
> Good catch indeed!
Thanks: fixed in v5.
>>
>> > + case __LINE__:
>>
>> The "case" should be further left, no?
>
> You're right!
>
> Thank you!
> Willy
Thank you for the review!