From: Stefan Sperling <stsp@stsp.name>
Subject: Re: plug small leak in match_loose_object
To: Omar Polo <op@omarpolo.com>
Cc: gameoftrees@openbsd.org
Date: Fri, 2 Sep 2022 10:00:56 +0200

On Fri, Sep 02, 2022 at 09:56:20AM +0200, Omar Polo wrote:
> 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?

Yes, this is much better. ok

> 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;
>  }
> 
>