Hello,

Han Boetes <hboetes@gmail.com> wrote:
> Hello all,
> 
> While reviewing the fix Omar applied to echo.c, I noticed the same 
> pattern in two more places.

Thanks =)

I'm tempted to just switch the snprintf in the first hunk to asprintf()
instead, which yields a smaller diff.  The only downside that I see is
that we could process strings longer than NFILEN+2, but I don't think
it's a problem.  See diff below, I haven't committed it yet.

> The following patch aims to fix these problems. Sorry for the mangling, 
> no idea how to fix that.

No problem!  For diffs as small as these, I can manually apply them, and
it's not a problem for me to do so as part of the review.  For bigger
diffs however it will be a problem ^^"

From the headers it appears that you're using thunderbird, I haven't
used it personally so can't really help, but there is this doc page from
the linux kernel folks which might be helpful:

https://www.kernel.org/doc/html/latest/process/email-clients.html#thunderbird-gui

HTH


Cheers,
Omar Polo


diff /home/op/w/mg
path + /home/op/w/mg
commit - 666364bbae2b0db5a50aeaabb6a79a3ecc285f57
blob - fd44a8bcf32458905ae91e6e875abdb723533fd8
file + fileio.c
--- fileio.c
+++ fileio.c
@@ -439,7 +439,6 @@ make_file_list(char *buf)
 	DIR		*dirp;
 	struct dirent	*dent;
 	struct list	*last, *current;
-	char		 fl_name[NFILEN + 2];
 	char		 prefixx[NFILEN + 1];
 
 	/*
@@ -541,14 +540,13 @@ make_file_list(char *buf)
 			closedir(dirp);
 			return (NULL);
 		}
-		ret = snprintf(fl_name, sizeof(fl_name),
+		ret = asprintf(&current->l_name,
 		    "%s%s%s", prefixx, dent->d_name, isdir ? "/" : "");
-		if (ret < 0 || ret >= sizeof(fl_name)) {
+		if (ret == -1) {
 			free(current);
 			continue;
 		}
 		current->l_next = last;
-		current->l_name = strdup(fl_name);
 		last = current;
 	}
 	closedir(dirp);
commit - 666364bbae2b0db5a50aeaabb6a79a3ecc285f57
blob - ae1c455edff7acaa51ba8c4acd1240f1f246e7c6
file + funmap.c
--- funmap.c
+++ funmap.c
@@ -311,7 +311,11 @@ complete_function_list(const char *fname)
 				free_file_list(head);
 				return (NULL);
 			}
-			el->l_name = strdup(fn->fn_name);
+			if ((el->l_name = strdup(fn->fn_name)) == NULL) {
+				free(el);
+				free_file_list(head);
+				return (NULL);
+			}
 			el->l_next = head;
 			head = el;
 		}