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

2012-09-21 Thread Michael Haggerty
On 09/20/2012 09:46 PM, Adam Spiers wrote:
 Signed-off-by: Adam Spiers g...@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
mhag...@alum.mit.edu
http://softwareswirl.blogspot.com/
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html


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

2012-09-21 Thread Junio C Hamano
Michael Haggerty mhag...@alum.mit.edu writes:

 On 09/20/2012 09:46 PM, Adam Spiers wrote:
 Signed-off-by: Adam Spiers g...@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.

All good points, including the last one.  Thanks.
--
To unsubscribe from this list: send the line unsubscribe git in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html