Re: [PATCH v4 3/8] object-store: add function to free object_info contents

[Junio C Hamano <gitster@pobox.com>]

Calvin Wan <calvinwan@google.com> writes:

> Introduce free_object_info_contents.
>
> ---
>  object-file.c  | 16 ++++++++++++++++
>  object-store.h |  3 +++
>  2 files changed, 19 insertions(+)
>
> diff --git a/object-file.c b/object-file.c
> index 5ffbf3d4fd..34a6e34adf 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -2659,3 +2659,19 @@ int read_loose_object(const char *path,
>  		munmap(map, mapsize);
>  	return ret;
>  }
> +
> +void free_object_info_contents(struct object_info *object_info)
> +{
> +	if (!object_info)
> +		return;

This is OK, but ...


> +	if (object_info->typep)
> +		free(object_info->typep);
> +	if (object_info->sizep)
> +		free(object_info->sizep);
> +	if (object_info->disk_sizep)
> +		free(object_info->disk_sizep);
> +	if (object_info->delta_base_oid)
> +		free(object_info->delta_base_oid);
> +	if (object_info->type_name)
> +		free(object_info->type_name);

	if (PTR)
		free(PTR);

can and should be written as

	free(PTR);

If we are reusing object_info after calling this function, we
_might_ want to use FREE_AND_NULL() instead of free().

> +}
> \ No newline at end of file

Let's avoid such incomplete lines.

> diff --git a/object-store.h b/object-store.h
> index bd2322ed8c..840e04b56f 100644
> --- a/object-store.h
> +++ b/object-store.h
> @@ -533,4 +533,7 @@ int for_each_object_in_pack(struct packed_git *p,
>  int for_each_packed_object(each_packed_object_fn, void *,
>  			   enum for_each_object_flags flags);
>  
> +/* Free pointers inside of object_info, but not object_info itself */
> +void free_object_info_contents(struct object_info *object_info);
> +
>  #endif /* OBJECT_STORE_H */