[PATCH] Free the results of scandir()

[PATCH] Free the results of scandir()

[Ethan Glasser-Camp]

From: Ethan Glasser-Camp <ethan@betacantrips.com>

scandir() returns "strings allocated via malloc(3)" which are then
"collected in array namelist which is allocated via
malloc(3)". Currently we just free the array namelist. Instead, free
all the entries of namelist, and then free namelist.

entry only points to elements of namelist, so we don't free it
separately.
---

This should fix a minor memory leak in notmuch-new. Please confirm I'm
reading the manpage correctly ;)

 notmuch-new.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index a569a54..c536873 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -559,12 +559,14 @@ add_files_recursive (notmuch_database_t *notmuch,
   DONE:
     if (next)
 	talloc_free (next);
-    if (entry)
-	free (entry);
     if (dir)
 	closedir (dir);
-    if (fs_entries)
+    if (fs_entries){
+	for (i = 0; i < num_fs_entries; i++){
+	    free (fs_entries[i]);
+	}
 	free (fs_entries);
+    }
     if (db_subdirs)
 	notmuch_filenames_destroy (db_subdirs);
     if (db_files)
-- 
1.7.5.4

Re: [PATCH] Free the results of scandir()

[Jani Nikula]

On Mon,  6 Feb 2012 17:02:49 -0500, Ethan Glasser-Camp <glasse@cs.rpi.edu> wrote:
> From: Ethan Glasser-Camp <ethan@betacantrips.com>
> 
> scandir() returns "strings allocated via malloc(3)" which are then
> "collected in array namelist which is allocated via
> malloc(3)". Currently we just free the array namelist. Instead, free
> all the entries of namelist, and then free namelist.
> 
> entry only points to elements of namelist, so we don't free it
> separately.
> ---
> 
> This should fix a minor memory leak in notmuch-new. Please confirm I'm
> reading the manpage correctly ;)

It looks right, good catch! Please do also fix the other scandir() usage
in count_files().

BR,
Jani.


> 
>  notmuch-new.c |    8 +++++---
>  1 files changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/notmuch-new.c b/notmuch-new.c
> index a569a54..c536873 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -559,12 +559,14 @@ add_files_recursive (notmuch_database_t *notmuch,
>    DONE:
>      if (next)
>  	talloc_free (next);
> -    if (entry)
> -	free (entry);
>      if (dir)
>  	closedir (dir);
> -    if (fs_entries)
> +    if (fs_entries){
> +	for (i = 0; i < num_fs_entries; i++){
> +	    free (fs_entries[i]);
> +	}
>  	free (fs_entries);
> +    }
>      if (db_subdirs)
>  	notmuch_filenames_destroy (db_subdirs);
>      if (db_files)
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

[PATCH] Free the results of scandir()

[Ethan Glasser-Camp]

From: Ethan Glasser-Camp <ethan@betacantrips.com>

scandir() returns "strings allocated via malloc(3)" which are then
"collected in array namelist which is allocated via
malloc(3)". Currently we just free the array namelist. Instead, free
all the entries of namelist, and then free namelist.

entry only points to elements of namelist, so we don't free it
separately.
---

v3: I'm still learning the house style. Thanks Dmitry.

 notmuch-new.c |   16 ++++++++++------
 1 files changed, 10 insertions(+), 6 deletions(-)

diff --git a/notmuch-new.c b/notmuch-new.c
index a569a54..8dbebb3 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -559,12 +559,14 @@ add_files_recursive (notmuch_database_t *notmuch,
   DONE:
     if (next)
 	talloc_free (next);
-    if (entry)
-	free (entry);
     if (dir)
 	closedir (dir);
-    if (fs_entries)
+    if (fs_entries) {
+	for (i = 0; i < num_fs_entries; i++)
+	    free (fs_entries[i]);
+
 	free (fs_entries);
+    }
     if (db_subdirs)
 	notmuch_filenames_destroy (db_subdirs);
     if (db_files)
@@ -704,10 +706,12 @@ count_files (const char *path, int *count)
     }
 
   DONE:
-    if (entry)
-	free (entry);
-    if (fs_entries)
+    if (fs_entries) {
+	for (i = 0; i < num_fs_entries; i++)
+	    free (fs_entries[i]);
+
         free (fs_entries);
+    }
 }
 
 static void
-- 
1.7.5.4

Re: [PATCH] Free the results of scandir()

[Dmitry Kurochkin]

On Tue,  7 Feb 2012 05:05:03 -0500, Ethan Glasser-Camp <glasse@cs.rpi.edu> wrote:
> From: Ethan Glasser-Camp <ethan@betacantrips.com>
> 
> scandir() returns "strings allocated via malloc(3)" which are then
> "collected in array namelist which is allocated via
> malloc(3)". Currently we just free the array namelist. Instead, free
> all the entries of namelist, and then free namelist.
> 
> entry only points to elements of namelist, so we don't free it
> separately.
> ---
> 
> v3: I'm still learning the house style. Thanks Dmitry.
> 

Looks good.

Please use --subject-prefix='PATCH vN' parameter when sending new
versions of patches.  Also, sending new versions as replies to the first
email in the original thread makes it easier to track.

Regards,
  Dmitry

>  notmuch-new.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/notmuch-new.c b/notmuch-new.c
> index a569a54..8dbebb3 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -559,12 +559,14 @@ add_files_recursive (notmuch_database_t *notmuch,
>    DONE:
>      if (next)
>  	talloc_free (next);
> -    if (entry)
> -	free (entry);
>      if (dir)
>  	closedir (dir);
> -    if (fs_entries)
> +    if (fs_entries) {
> +	for (i = 0; i < num_fs_entries; i++)
> +	    free (fs_entries[i]);
> +
>  	free (fs_entries);
> +    }
>      if (db_subdirs)
>  	notmuch_filenames_destroy (db_subdirs);
>      if (db_files)
> @@ -704,10 +706,12 @@ count_files (const char *path, int *count)
>      }
>  
>    DONE:
> -    if (entry)
> -	free (entry);
> -    if (fs_entries)
> +    if (fs_entries) {
> +	for (i = 0; i < num_fs_entries; i++)
> +	    free (fs_entries[i]);
> +
>          free (fs_entries);
> +    }
>  }
>  
>  static void
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch

Re: [PATCH] Free the results of scandir()

[Ethan Glasser-Camp]

On 02/07/2012 05:10 AM, Dmitry Kurochkin wrote:
> Please use --subject-prefix='PATCH vN' parameter when sending new
> versions of patches.  Also, sending new versions as replies to the first
> email in the original thread makes it easier to track.

Oops! Thanks again.

Ethan

Re: [PATCH] Free the results of scandir()

[Tomi Ollila]

[-- Attachment #1: Type: text/plain, Size: 499 bytes --]

On Tue,  7 Feb 2012 05:05:03 -0500, Ethan Glasser-Camp <glasse@cs.rpi.edu> wrote:
> From: Ethan Glasser-Camp <ethan@betacantrips.com>
> 
> scandir() returns "strings allocated via malloc(3)" which are then
> "collected in array namelist which is allocated via
> malloc(3)". Currently we just free the array namelist. Instead, free
> all the entries of namelist, and then free namelist.
> 
> entry only points to elements of namelist, so we don't free it
> separately.
> ---

+1

Tomi

[-- Attachment #2: Type: application/pgp-signature, Size: 188 bytes --]

Re: [PATCH] Free the results of scandir()

[David Bremner]

On Tue,  7 Feb 2012 05:05:03 -0500, Ethan Glasser-Camp <glasse@cs.rpi.edu> wrote:
> From: Ethan Glasser-Camp <ethan@betacantrips.com>
> 
> scandir() returns "strings allocated via malloc(3)" which are then
> "collected in array namelist which is allocated via
> malloc(3)". Currently we just free the array namelist. Instead, free
> all the entries of namelist, and then free namelist.

pushed, thanks

d