lsmmc: rework string trimming

Commit Message

Michael Heimpold Nov. 4, 2018, 1:21 p.m. UTC

gcc8 complains with:

lsmmc.c: In function 'read_file':
lsmmc.c:356:3: error: 'strncpy' accessing 4096 bytes at \
  offsets 0 and 1 overlaps 4095 bytes at offset 1 [-Werror=restrict]
   strncpy(&line[0], &line[1], sizeof(line));
   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

The man page for strncpy() does not clearly state whether it's safe to
use overlapping memory regions while copying; for the similar strcpy()
this is prohibited, so I would also assume this for strncpy().

So let's refactor this part.

Reported-by: Simon Tretter
Signed-off-by: Michael Heimpold <[e-mail]>
---
 lsmmc.c | 25 +++++++++++++++----------
 1 file changed, 15 insertions(+), 10 deletions(-)

Comments

Uwe Kleine-König Nov. 4, 2018, 4:10 p.m. UTC | #1

On Sun, Nov 04, 2018 at 02:21:49PM +0100, Michael Heimpold wrote:
> gcc8 complains with:
> 
> lsmmc.c: In function 'read_file':
> lsmmc.c:356:3: error: 'strncpy' accessing 4096 bytes at \
>   offsets 0 and 1 overlaps 4095 bytes at offset 1 [-Werror=restrict]
>    strncpy(&line[0], &line[1], sizeof(line));
>    ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

You're the third person (after Hans-Christian Noren Egtvedt and Rob
Mosher) to send me a patch addressing this issue.

@Chris: Can you please look into this that people stop patching the same
issue again and again and also to send me the patches instead of you?

Best regards
Uwe

Patch

diff --git a/lsmmc.c b/lsmmc.c
index c4faa00..d352e74 100644
--- a/lsmmc.c
+++ b/lsmmc.c
@@ -316,8 +316,9 @@  int parse_ids(struct config *config)
 /* MMC/SD file parsing functions */
 char *read_file(char *name)
 {
-	char *preparsed;
+	char *start, *end;
 	char line[4096];
+	int len;
 	FILE *f;
 
 	f = fopen(name, "r");
@@ -326,8 +327,8 @@  char *read_file(char *name)
 		return NULL;
 	}
 
-	preparsed = fgets(line, sizeof(line), f);
-	if (!preparsed) {
+	start = fgets(line, sizeof(line), f);
+	if (!start) {
 		if (ferror(f))
 			fprintf(stderr, "Could not read MMC/SD file '%s'.\n",
 				name);
@@ -347,15 +348,19 @@  char *read_file(char *name)
 		return NULL;
 	}
 
-	line[sizeof(line) - 1] = '\0';
+	/* skip leading white-space */
+	while (isspace(*start))
+		start++;
 
-	while (isspace(line[strlen(line) - 1]))
-		line[strlen(line) - 1] = '\0';
-
-	while (isspace(line[0]))
-		strncpy(&line[0], &line[1], sizeof(line));
+	/* trim trailing white-space */
+	len = strlen(start);
+	if (len) {
+		end = &start[len - 1];
+		while (isspace(*end))
+			*end-- = '\0';
+	}
 
-	return strdup(line);
+	return strdup(start);
 }
 
 /* Hexadecimal string parsing functions */