Re: [PATCH v2 13/14] Provide free_directory() for reclaiming dir_struct memory

From: Michael Haggerty <mhagger@alum.mit.edu>
Date: Sep 21, 2012, 08:03 UTC
Message-ID: <505C1F69.1050508@alum.mit.edu>
In-Reply-To: <1348170383-15751-14-git-send-email-git@adamspiers.org>

On 09/20/2012 09:46 PM, Adam Spiers wrote:

Show 31 quoted lines

> Signed-off-by: Adam Spiers <git@adamspiers.org>
> ---
>  Documentation/technical/api-directory-listing.txt |  2 ++
>  dir.c                                             | 23 +++++++++++++++++++++--
>  dir.h                                             |  1 +
>  3 files changed, 24 insertions(+), 2 deletions(-)
> 
> diff --git a/Documentation/technical/api-directory-listing.txt b/Documentation/technical/api-directory-listing.txt
> index 944fc39..e339c18 100644
> --- a/Documentation/technical/api-directory-listing.txt
> +++ b/Documentation/technical/api-directory-listing.txt
> @@ -79,4 +79,6 @@ marked. If you to exclude files, make sure you have loaded index first.
>  
>  * Use `dir.entries[]`.
>  
> +* Call `free_directory()` when none of the contained elements are no longer in use.
> +
>  (JC)
> [...]
> diff --git a/dir.h b/dir.h
> index ebb0367..7da29da 100644
> --- a/dir.h
> +++ b/dir.h
> @@ -128,6 +128,7 @@ extern void add_excludes_from_file(struct dir_struct *, const char *fname);
>  extern void add_exclude(const char *string, const char *base,
>  			int baselen, struct exclude_list *el, const char *src, int srcpos);
>  extern void free_excludes(struct exclude_list *el);
> +extern void free_directory(struct dir_struct *dir);
>  extern int file_exists(const char *);
>  
>  extern int is_inside_dir(const char *dir);

With I see a function like this, the first question in my head is always "does it also free(dir), or does it only free the substructures, leaving dir empty but allocated?" There should be a comment documenting the behavior. I also find it helpful if a function that frees the top-level structure has "free" in the name, while a function that only empties the top-level structure without freeing it *not* have free in the name (e.g., "clear_directory()"). But maybe that's just me.

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu
http://softwareswirl.blogspot.com/