[LTP] [PATCH 0/8] Fix uninitialized var errors

[LTP] [PATCH 0/8] Fix uninitialized var errors

[Richard Palethorpe]

Hello,

It would be nice if we could set --Werror=uninitialized by
default. Uninitialized variables can result in nasty compiler
behaviour. You may expect it just to result in garbage data being
used. The result being a crash. However as we compile with -O2 this is
not the case. Often the compiler chooses values for the variable which
will eliminate code. This often means the values are valid. The result
being a program which appears to execute correctly on the
surface. Meanwhile underneath it is doing very little.

In a lot of cases the compiler is simply confused and gives an error,
but there is no undefined behaviour. This is often caused by tst_brk
which is the source of much confusion. Most likely there should be two
versions of tst_brk. One which never returns and is used by test
authors and one which can return, but is usually only used in library
functions which may be called from cleanup.

Richard Palethorpe (8):
  ipc: Fix uninitialized var error by removing useless arg
  ftest: Fix uninitialized var error
  hotplug: Fix uninitialized var
  dio: Fix uninitialized var error
  ksmg01: Fix uninitialized var error
  shmat01: Fix uninitialized var error
  crash02: Save actual child PID instead of uninitialized variable
  locktests: Fix uninitialized var error

 libs/libltpipc/libipc.c                            | 4 +---
 testcases/kernel/fs/ftest/ftest01.c                | 3 +--
 testcases/kernel/fs/ftest/ftest05.c                | 3 +--
 testcases/kernel/hotplug/memory_hotplug/commands.c | 7 +------
 testcases/kernel/io/ltp-aiodio/dio_append.c        | 2 +-
 testcases/kernel/io/ltp-aiodio/dio_sparse.c        | 2 +-
 testcases/kernel/io/ltp-aiodio/dio_truncate.c      | 4 ++--
 testcases/kernel/logging/kmsg/kmsg01.c             | 2 +-
 testcases/kernel/syscalls/ipc/semctl/semctl01.c    | 4 +---
 testcases/kernel/syscalls/ipc/shmat/shmat01.c      | 2 +-
 testcases/misc/crash/crash02.c                     | 6 +++---
 testcases/network/nfsv4/locks/locktests.c          | 2 +-
 12 files changed, 15 insertions(+), 26 deletions(-)

-- 
2.31.1

[LTP] [PATCH 1/8] ipc: Fix uninitialized var error by removing useless arg

[Richard Palethorpe]

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 libs/libltpipc/libipc.c                         | 4 +---
 testcases/kernel/syscalls/ipc/semctl/semctl01.c | 4 +---
 2 files changed, 2 insertions(+), 6 deletions(-)

diff --git a/libs/libltpipc/libipc.c b/libs/libltpipc/libipc.c
index d94880f54..c2ecbf02d 100644
--- a/libs/libltpipc/libipc.c
+++ b/libs/libltpipc/libipc.c
@@ -122,13 +122,11 @@ void init_buf(MSGBUF * m_buf, int type, int size)
  */
 void rm_sema(int sem_id)
 {
-	union semun arr;
-
 	if (sem_id == -1) {	/* no semaphore to remove */
 		return;
 	}
 
-	if (semctl(sem_id, 0, IPC_RMID, arr) == -1) {
+	if (semctl(sem_id, 0, IPC_RMID) == -1) {
 		tst_resm(TINFO, "WARNING: semaphore deletion failed.");
 		tst_resm(TINFO, "This could lead to IPC resource problems.");
 		tst_resm(TINFO, "id = %d", sem_id);
diff --git a/testcases/kernel/syscalls/ipc/semctl/semctl01.c b/testcases/kernel/syscalls/ipc/semctl/semctl01.c
index b12385970..ff691adf8 100644
--- a/testcases/kernel/syscalls/ipc/semctl/semctl01.c
+++ b/testcases/kernel/syscalls/ipc/semctl/semctl01.c
@@ -196,10 +196,8 @@ static void func_sall(void)
 
 static void func_sval(void)
 {
-	int semv;
-	union semun arr;
+	int semv = SAFE_SEMCTL(sem_id, 4, GETVAL);
 
-	semv = SAFE_SEMCTL(sem_id, 4, GETVAL, arr);
 	if (semv != INCVAL)
 		tst_res(TFAIL, "semaphore value is not what was set");
 	else
-- 
2.31.1

[LTP] [PATCH 2/8] ftest: Fix uninitialized var error

[Richard Palethorpe]

Mark misc_intvl as const which allows the compiler to eliminate some
confusing branches. This is a minimal change to fix the error.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/fs/ftest/ftest01.c | 3 +--
 testcases/kernel/fs/ftest/ftest05.c | 3 +--
 2 files changed, 2 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/fs/ftest/ftest01.c b/testcases/kernel/fs/ftest/ftest01.c
index afad18095..31203d689 100644
--- a/testcases/kernel/fs/ftest/ftest01.c
+++ b/testcases/kernel/fs/ftest/ftest01.c
@@ -83,7 +83,7 @@ static void term(int sig);
 static int csize;		/* chunk size */
 static int iterations;		/* # total iterations */
 static int max_size;		/* max file size */
-static int misc_intvl;		/* for doing misc things; 0 ==> no */
+static const int misc_intvl = 10;		/* for doing misc things; 0 ==> no */
 static int nchild;		/* how many children */
 static int fd;			/* file descriptor used by child */
 static int parent_pid;
@@ -139,7 +139,6 @@ static void setup(void)
 	nchild = 5;
 	csize = K_2;		/* should run with 1, 2, and 4 K sizes */
 	max_size = K_1 * K_1;
-	misc_intvl = 10;
 
 	if (sigset(SIGTERM, term) == SIG_ERR) {
 		tst_brkm(TBROK | TERRNO, NULL, "sigset failed");
diff --git a/testcases/kernel/fs/ftest/ftest05.c b/testcases/kernel/fs/ftest/ftest05.c
index 0bd32e4f6..8d8e6d497 100644
--- a/testcases/kernel/fs/ftest/ftest05.c
+++ b/testcases/kernel/fs/ftest/ftest05.c
@@ -87,7 +87,7 @@ static void cleanup(void);
 static int csize;		/* chunk size */
 static int iterations;		/* # total iterations */
 static off64_t max_size;	/* max file size */
-static int misc_intvl;		/* for doing misc things; 0 ==> no */
+static const int misc_intvl = 10;		/* for doing misc things; 0 ==> no */
 static int nchild;		/* how many children */
 static int fd;			/* file descriptor used by child */
 static int parent_pid;
@@ -147,7 +147,6 @@ static void setup(void)
 	nchild = 5;
 	csize = K_2;		/* should run with 1, 2, and 4 K sizes */
 	max_size = K_1 * K_1;
-	misc_intvl = 10;
 
 	if (sigset(SIGTERM, term) == SIG_ERR) {
 		tst_brkm(TBROK | TERRNO, NULL,
-- 
2.31.1

[LTP] [PATCH 3/8] hotplug: Fix uninitialized var

[Richard Palethorpe]

This assumes glctx is the correct thing to use as in every other
function.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/hotplug/memory_hotplug/commands.c | 7 +------
 1 file changed, 1 insertion(+), 6 deletions(-)

diff --git a/testcases/kernel/hotplug/memory_hotplug/commands.c b/testcases/kernel/hotplug/memory_hotplug/commands.c
index 6655d3553..78f46fbca 100644
--- a/testcases/kernel/hotplug/memory_hotplug/commands.c
+++ b/testcases/kernel/hotplug/memory_hotplug/commands.c
@@ -438,16 +438,11 @@ static int get_arg_nodeid_list(char *args, unsigned int *list)
  */
 static int get_current_nodeid_list(unsigned int *fromids)
 {
-	/*
-	 * FIXME (garrcoop): gcp is uninitialized and shortly hereafter used in
-	 * an initialization statement..... UHHHHHHH... test writer fail?
-	 */
-	glctx_t *gcp;
+	glctx_t *gcp = &glctx;
 	nodemask_t my_allowed_nodes;
 	int nr_nodes = 0, max_node = gcp->numa_max_node;
 	int node;
 
-	gcp = &glctx;
 	my_allowed_nodes = numa_get_membind_compat();
 	for (node = 0; node <= max_node; ++node) {
 		if (nodemask_isset(&my_allowed_nodes, node))
-- 
2.31.1

[LTP] [PATCH 4/8] dio: Fix uninitialized var error

[Richard Palethorpe]

The compiler does not know that we will return if posix_memalign fails
to set bufptr.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/io/ltp-aiodio/dio_append.c   | 2 +-
 testcases/kernel/io/ltp-aiodio/dio_sparse.c   | 2 +-
 testcases/kernel/io/ltp-aiodio/dio_truncate.c | 4 ++--
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/testcases/kernel/io/ltp-aiodio/dio_append.c b/testcases/kernel/io/ltp-aiodio/dio_append.c
index 3f0ed29d5..b1b4dc039 100644
--- a/testcases/kernel/io/ltp-aiodio/dio_append.c
+++ b/testcases/kernel/io/ltp-aiodio/dio_append.c
@@ -78,7 +78,7 @@ int read_eof(char *filename)
 void dio_append(char *filename)
 {
 	int fd;
-	void *bufptr;
+	void *bufptr = NULL;
 	int i;
 	int w;
 
diff --git a/testcases/kernel/io/ltp-aiodio/dio_sparse.c b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
index 67b338b3f..3f44e92ea 100644
--- a/testcases/kernel/io/ltp-aiodio/dio_sparse.c
+++ b/testcases/kernel/io/ltp-aiodio/dio_sparse.c
@@ -57,7 +57,7 @@ int TST_TOTAL = 1;
  */
 int dio_sparse(int fd, int align, int writesize, int filesize, int offset)
 {
-	void *bufptr;
+	void *bufptr = NULL;
 	int i, w;
 
 	TEST(posix_memalign(&bufptr, align, writesize));
diff --git a/testcases/kernel/io/ltp-aiodio/dio_truncate.c b/testcases/kernel/io/ltp-aiodio/dio_truncate.c
index 7d466dc20..27cf01525 100644
--- a/testcases/kernel/io/ltp-aiodio/dio_truncate.c
+++ b/testcases/kernel/io/ltp-aiodio/dio_truncate.c
@@ -69,7 +69,7 @@ int dio_read(char *filename)
 {
 	int fd;
 	int r;
-	void *bufptr;
+	void *bufptr = NULL;
 
 	TEST(posix_memalign(&bufptr, 4096, 64 * 1024));
 	if (TEST_RETURN) {
@@ -105,7 +105,7 @@ int dio_read(char *filename)
 void dio_append(char *filename, int fill)
 {
 	int fd;
-	void *bufptr;
+	void *bufptr = NULL;
 	int i;
 	int w;
 
-- 
2.31.1

[LTP] [PATCH 5/8] ksmg01: Fix uninitialized var error

[Richard Palethorpe]

The compiler is confused by tst_brk which it thinks will
return. Setting first_seqno silences the error.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/logging/kmsg/kmsg01.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/logging/kmsg/kmsg01.c b/testcases/kernel/logging/kmsg/kmsg01.c
index 34c6073c6..bf2de5741 100644
--- a/testcases/kernel/logging/kmsg/kmsg01.c
+++ b/testcases/kernel/logging/kmsg/kmsg01.c
@@ -384,7 +384,7 @@ static void test_messages_overwritten(void)
 {
 	int i, fd;
 	char msg[MAX_MSGSIZE];
-	unsigned long first_seqno, seqno;
+	unsigned long first_seqno = 0, seqno;
 	char filler_str[] = "<7>"MSG_PREFIX"FILLER MESSAGE TO OVERWRITE OTHERS\n";
 
 	/* Keep injecting messages until we overwrite first one.
-- 
2.31.1

[LTP] [PATCH 6/8] shmat01: Fix uninitialized var error

[Richard Palethorpe]

The compiler is confused by SAFE_WAITPID which it thinks will return
after an error. Setting status to zero silences the error.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/kernel/syscalls/ipc/shmat/shmat01.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/kernel/syscalls/ipc/shmat/shmat01.c b/testcases/kernel/syscalls/ipc/shmat/shmat01.c
index f75914799..606bdd154 100644
--- a/testcases/kernel/syscalls/ipc/shmat/shmat01.c
+++ b/testcases/kernel/syscalls/ipc/shmat/shmat01.c
@@ -82,7 +82,7 @@ static void verify_shmat(unsigned int n)
 {
 	int *addr;
 	pid_t pid;
-	int status;
+	int status = 0;
 	struct shmid_ds buf;
 
 	struct test_case_t *tc = &tcases[n];
-- 
2.31.1

[LTP] [PATCH 7/8] crash02: Save actual child PID instead of uninitialized variable

[Richard Palethorpe]

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/misc/crash/crash02.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/testcases/misc/crash/crash02.c b/testcases/misc/crash/crash02.c
index e46c2aa66..66f308b70 100644
--- a/testcases/misc/crash/crash02.c
+++ b/testcases/misc/crash/crash02.c
@@ -197,9 +197,8 @@ void monitor_fcn(int sig)
 void badboy_fork(void)
 {
 	int status, pid;
-	pid_t child;
-	child = fork();
-	badboy_pid = status;
+	pid_t child = fork();
+
 	switch (child) {
 	case -1:
 		perror("fork");
@@ -211,6 +210,7 @@ void badboy_fork(void)
 #endif
 		exit(0);
 	default:
+		badboy_pid = child;
 		if (verbose_level > 3)
 			printf("badboy pid = %d\n", badboy_pid);
 
-- 
2.31.1

[LTP] [PATCH 8/8] locktests: Fix uninitialized var error

[Richard Palethorpe]

The compiler does not appear to understand that the initial
state (SELECT) will always initialize tLock in the first
iteration. Initializing tLock to zero suppresses the error.

Signed-off-by: Richard Palethorpe <rpalethorpe@suse.com>
---
 testcases/network/nfsv4/locks/locktests.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/testcases/network/nfsv4/locks/locktests.c b/testcases/network/nfsv4/locks/locktests.c
index d2c766bc1..54faca3ec 100644
--- a/testcases/network/nfsv4/locks/locktests.c
+++ b/testcases/network/nfsv4/locks/locktests.c
@@ -580,7 +580,7 @@ int master(void)
 	char dbg[16];
 #endif
 	struct flock request;
-	struct s_test tLock;
+	struct s_test tLock = { 0 };
 	enum state_t state;
 	int offset;
 	/* A test sentence written in the file */
-- 
2.31.1

[LTP] [PATCH 0/8] Fix uninitialized var errors

[Cyril Hrubis]

Hi!
> In a lot of cases the compiler is simply confused and gives an error,
> but there is no undefined behaviour. This is often caused by tst_brk
> which is the source of much confusion. Most likely there should be two
> versions of tst_brk. One which never returns and is used by test
> authors and one which can return, but is usually only used in library
> functions which may be called from cleanup.

Having two tst_brk() wouldn't solve much since all SAFE_MACROS() still
needs to use the one that may return.

So as far as I can tell it's easier to initialize variables to 0 in the
few cases where compiler ends up confused.

Anyways whole patchset pushed, thanks.

-- 
Cyril Hrubis
chrubis@suse.cz