2023-08-03 08:15:49

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 00/14] tools/nolibc: enable compiler warnings

To help the developers to avoid mistakes and keep the code smaller let's
enable compiler warnings.

I stuck with __attribute__((unused)) over __maybe_unused in
nolibc-test.c for consistency with nolibc proper.
If we want to add a define it needs to be added twice once for nolibc
proper and once for nolibc-test otherwise libc-test wouldn't build
anymore.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
Changes in v3:
- Make getpagesize() return "int"
- Simplify validation of read() return value
- Don't make functions static that are to be used as breakpoints
- Drop -s from LDFLAGS
- Use proper types for read()/write() return values
- Fix unused parameter warnings in new setvbuf()
- Link to v2: https://lore.kernel.org/r/[email protected]

Changes in v2:
- Don't drop unused test helpers, mark them as __attribute__((unused))
- Make some function in nolibc-test static
- Also handle -W and -Wextra
- Link to v1: https://lore.kernel.org/r/[email protected]

---
Thomas Weißschuh (14):
tools/nolibc: drop unused variables
tools/nolibc: fix return type of getpagesize()
tools/nolibc: setvbuf: avoid unused parameter warnings
tools/nolibc: sys: avoid implicit sign cast
tools/nolibc: stdint: use int for size_t on 32bit
selftests/nolibc: drop unused variables
selftests/nolibc: mark test helpers as potentially unused
selftests/nolibc: make functions static if possible
selftests/nolibc: avoid unused parameter warnings
selftests/nolibc: avoid sign-compare warnings
selftests/nolibc: use correct return type for read() and write()
selftests/nolibc: prevent out of bounds access in expect_vfprintf
selftests/nolibc: don't strip nolibc-test
selftests/nolibc: enable compiler warnings

tools/include/nolibc/stdint.h | 4 +
tools/include/nolibc/stdio.h | 5 +-
tools/include/nolibc/sys.h | 7 +-
tools/testing/selftests/nolibc/Makefile | 4 +-
tools/testing/selftests/nolibc/nolibc-test.c | 111 ++++++++++++++++-----------
5 files changed, 80 insertions(+), 51 deletions(-)
---
base-commit: bc87f9562af7b2b4cb07dcaceccfafcf05edaff8
change-id: 20230731-nolibc-warnings-c6e47284ac03

Best regards,
--
Thomas Weißschuh <[email protected]>



2023-08-03 08:16:02

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 01/14] tools/nolibc: drop unused variables

Nobody needs it, get rid of it.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/include/nolibc/sys.h | 1 -
1 file changed, 1 deletion(-)

diff --git a/tools/include/nolibc/sys.h b/tools/include/nolibc/sys.h
index 56f63eb48a1b..e12dd962c578 100644
--- a/tools/include/nolibc/sys.h
+++ b/tools/include/nolibc/sys.h
@@ -738,7 +738,6 @@ static __attribute__((unused))
int open(const char *path, int flags, ...)
{
mode_t mode = 0;
- int ret;

if (flags & O_CREAT) {
va_list args;

--
2.41.0


2023-08-03 08:27:27

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 06/14] selftests/nolibc: drop unused variables

These got copied around as new testcases where created.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 4 ----
1 file changed, 4 deletions(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 52489455add8..cff441c17f3e 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -908,9 +908,7 @@ int run_syscall(int min, int max)
int run_stdlib(int min, int max)
{
int test;
- int tmp;
int ret = 0;
- void *p1, *p2;

for (test = min; test >= 0 && test <= max; test++) {
int llen = 0; /* line length */
@@ -1051,9 +1049,7 @@ static int expect_vfprintf(int llen, size_t c, const char *expected, const char
static int run_vfprintf(int min, int max)
{
int test;
- int tmp;
int ret = 0;
- void *p1, *p2;

for (test = min; test >= 0 && test <= max; test++) {
int llen = 0; /* line length */

--
2.41.0


2023-08-03 08:29:36

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 09/14] selftests/nolibc: avoid unused parameter warnings

This warning will be enabled later so avoid triggering it.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index ea420c794536..d6aa12c3f9ff 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1112,7 +1112,8 @@ static int smash_stack(void)
return 1;
}

-static int run_protection(int min, int max)
+static int run_protection(int min __attribute__((unused)),
+ int max __attribute__((unused)))
{
pid_t pid;
int llen = 0, status;

--
2.41.0


2023-08-03 09:51:37

by Thomas Weißschuh

[permalink] [raw]
Subject: [PATCH v3 12/14] selftests/nolibc: prevent out of bounds access in expect_vfprintf

If read() fails and returns -1 (or returns garbage for some other
reason) buf would be accessed out of bounds.
Only use the return value of read() after it has been validated.

Signed-off-by: Thomas Weißschuh <[email protected]>
---
tools/testing/selftests/nolibc/nolibc-test.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/tools/testing/selftests/nolibc/nolibc-test.c b/tools/testing/selftests/nolibc/nolibc-test.c
index 63f09800057d..0145a44af990 100644
--- a/tools/testing/selftests/nolibc/nolibc-test.c
+++ b/tools/testing/selftests/nolibc/nolibc-test.c
@@ -1055,7 +1055,6 @@ static int expect_vfprintf(int llen, int c, const char *expected, const char *fm
lseek(fd, 0, SEEK_SET);

r = read(fd, buf, sizeof(buf) - 1);
- buf[r] = '\0';

fclose(memfile);

@@ -1065,6 +1064,7 @@ static int expect_vfprintf(int llen, int c, const char *expected, const char *fm
return 1;
}

+ buf[r] = '\0';
llen += printf(" \"%s\" = \"%s\"", expected, buf);
ret = strncmp(expected, buf, c);


--
2.41.0


2023-08-05 17:42:36

by Willy Tarreau

[permalink] [raw]
Subject: Re: [PATCH v3 00/14] tools/nolibc: enable compiler warnings

On Thu, Aug 03, 2023 at 09:28:44AM +0200, Thomas Wei?schuh wrote:
> To help the developers to avoid mistakes and keep the code smaller let's
> enable compiler warnings.

All the series looks good, I've now queued it, thanks!

> I stuck with __attribute__((unused)) over __maybe_unused in
> nolibc-test.c for consistency with nolibc proper.
> If we want to add a define it needs to be added twice once for nolibc
> proper and once for nolibc-test otherwise libc-test wouldn't build
> anymore.

I tend to prefer to avoid spreading macros in nolibc itself unless
strictly necessary as we'd need to put them under a "nolibc" namespace
to avoid a risk of clash, and it becomes less interesting in terms of
number of characters saved per line when everything is prefixed with
"nolibc_" or so. It's convenient however when there are multiple
choices to be replicated at multiple places. So let's keep it like
this for now.

Cheers,
Willy