From: Omar Polo Subject: plug small leak in match_loose_object To: gameoftrees@openbsd.org Date: Fri, 02 Sep 2022 09:56:20 +0200 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; }