"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Omar Polo <op@omarpolo.com>
Subject:
plug small leak in match_loose_object
To:
gameoftrees@openbsd.org
Date:
Fri, 02 Sep 2022 09:56:20 +0200

Download raw body.

Thread
in the loop of match_loose_object we allocate a string per directory
entry.  In some case we properly free it before `continue' or `goto',
but in other cases we just forgot to do so.  Instead of littering
every jump with a free, what about doing something like the following?

diff /home/op/w/got
commit - cba69822a48bf0e7aeb5cf311aa42d7862859e97
path + /home/op/w/got
blob - 87c7abb573744c7b7f9b38f35d7550229267b418
file + lib/repository.c
--- lib/repository.c
+++ lib/repository.c
@@ -1710,7 +1710,7 @@ match_loose_object(struct got_object_id **unique_id, c
     struct got_repository *repo)
 {
 	const struct got_error *err = NULL;
-	char *path;
+	char *path, *id_str = NULL;
 	DIR *dir = NULL;
 	struct dirent *dent;
 	struct got_object_id id;
@@ -1730,9 +1730,11 @@ match_loose_object(struct got_object_id **unique_id, c
 		goto done;
 	}
 	while ((dent = readdir(dir)) != NULL) {
-		char *id_str;
 		int cmp;
 
+		free(id_str);
+		id_str = NULL;
+
 		if (strcmp(dent->d_name, ".") == 0 ||
 		    strcmp(dent->d_name, "..") == 0)
 			continue;
@@ -1750,10 +1752,8 @@ match_loose_object(struct got_object_id **unique_id, c
 		 * sorted order, so we must iterate over all of them.
 		 */
 		cmp = strncmp(id_str, id_str_prefix, strlen(id_str_prefix));
-		if (cmp != 0) {
-			free(id_str);
+		if (cmp != 0)
 			continue;
-		}
 
 		if (*unique_id == NULL) {
 			if (obj_type != GOT_OBJ_TYPE_ANY) {
@@ -1768,14 +1768,12 @@ match_loose_object(struct got_object_id **unique_id, c
 			*unique_id = got_object_id_dup(&id);
 			if (*unique_id == NULL) {
 				err = got_error_from_errno("got_object_id_dup");
-				free(id_str);
 				goto done;
 			}
 		} else {
 			if (got_object_id_cmp(*unique_id, &id) == 0)
 				continue; /* both packed and loose */
 			err = got_error(GOT_ERR_AMBIGUOUS_ID);
-			free(id_str);
 			goto done;
 		}
 	}
@@ -1786,6 +1784,7 @@ done:
 		free(*unique_id);
 		*unique_id = NULL;
 	}
+	free(id_str);
 	free(path);
 	return err;
 }