Re: Re: [hackers] [sbase][PATCH] Fixed a couple of uninitialised variable warnings from Clang

From: Evan Gates <evan.gates_AT_gmail.com>
Date: Mon, 06 Feb 2017 14:04:52 -0800

Hiltjo Posthuma <hiltjo_AT_codemadness.org> wrote:

> Probably a false positive, but for ed.c it looks ok to me.
>
> The join.c patch does not look ok to me (clang doesn't detect that
> eprintf() exists the program), so it's a false positive.

This is where the _Noreturn or noreturn attribute is handy.
Unfortunately that's C11.

It seems ed.c is the same problem, error() has a longjmp() instead of
return causing a false positive.

I do like getting rid of warnings but I'm not particularly fond of
changes purely for that reason.

As for join.c, if we do care about getting rid of warnings we can
rewrite to take advantage of the initialized values instead of assigning
again when s is "0". Although I'm not sure the intent is as clear this
way. Example patch below.

I'd like to hear more thoughts on this, if the community's response is
overwhelmingly one way or the other I'll gladly accept the changes.

----- 8< ----- 8< -----

From 5eae9c0bd10c6da57fcd8527e7bbc6c23cda4b08 Mon Sep 17 00:00:00 2001
From: Evan Gates <evan.gates_AT_gmail.com>
Date: Mon, 6 Feb 2017 13:50:23 -0800
Subject: [PATCH] Initialize to avoid false positive clang warning

clang doesn't recognize eprintf exits and warns about possibily
uninitialized values. Initialize the values and rearrange the
conditionals to avoid redundant assignment.
---
 join.c | 11 ++++-------
 1 file changed, 4 insertions(+), 7 deletions(-)
diff --git a/join.c b/join.c
index d3e2343..e62bb8f 100644
--- a/join.c
+++ b/join.c
_AT_@ -314,16 +314,13 @@ static struct spec *
 makespec(char *s)
 {
 	struct spec *sp;
-	int fileno;
-	size_t fldno;
+	int fileno = 0; /* default to "0", join field must be 0 and nothing else */
+	size_t fldno = 0;
 
-	if (!strcmp(s, "0")) {   /* join field must be 0 and nothing else */
-		fileno = 0;
-		fldno = 0;
-	} else if ((s[0] == '1' || s[0] == '2') && s[1] == '.') {
+	if ((s[0] == '1' || s[0] == '2') && s[1] == '.') {
 		fileno = s[0] - '0';
 		fldno = estrtonum(&s[2], 1, MIN(LLONG_MAX, SIZE_MAX)) - 1;
-	} else {
+	} else if (strcmp(s, "0")){
 		eprintf("%s: invalid format\n", s);
 	}
 
-- 
2.11.0
Received on Mon Feb 06 2017 - 23:04:52 CET

This archive was generated by hypermail 2.3.0 : Mon Feb 06 2017 - 23:12:18 CET