Re: [PATCH v2 13/14] Provide free_directory() for reclaiming dir_struct memory
Michael Haggerty writes: > On 09/20/2012 09:46 PM, Adam Spiers wrote: >> Signed-off-by: Adam Spiers >> --- >> 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
Re: [PATCH v2 13/14] Provide free_directory() for reclaiming dir_struct memory
On 09/20/2012 09:46 PM, Adam Spiers wrote: > Signed-off-by: Adam Spiers > --- > 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
