[PATCH v2] Free the results of scandir()

[PATCH v2] 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.
---

Fixes the other use of scandir in count_files. Thanks, Jani.

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

diff --git a/notmuch-new.c b/notmuch-new.c
index a569a54..e62560b 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 v2] Free the results of scandir()

[Jani Nikula]

On Tue,  7 Feb 2012 01:50:05 -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.

Looks good. Thanks, Ethan.

David, I'm not sure if this is worth a bugfix release on its own, but
definitely worth including if something else comes up.

id:"2flfwhht87d.fsf@diskless.uio.no" is a report about potential memory
leak in notmuch new from a few months back. CC Petter, the reporter.

> ---
> 
> Fixes the other use of scandir in count_files. Thanks, Jani.
> 
>  notmuch-new.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/notmuch-new.c b/notmuch-new.c
> index a569a54..e62560b 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 v2] Free the results of scandir()

[Dmitry Kurochkin]

Hi Ethan.

On Tue,  7 Feb 2012 01:50:05 -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.
> ---
> 
> Fixes the other use of scandir in count_files. Thanks, Jani.
> 

Few style comments below.  Otherwise looks good.

>  notmuch-new.c |   16 ++++++++++------
>  1 files changed, 10 insertions(+), 6 deletions(-)
> 
> diff --git a/notmuch-new.c b/notmuch-new.c
> index a569a54..e62560b 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){

Please add a space before '{'.

> +	for (i = 0; i < num_fs_entries; i++){
> +	    free (fs_entries[i]);
> +	}

Please remove "{}" around one line block.

>  	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]);
> +	}

Same two comments here.

Regards,
  Dmitry

>          free (fs_entries);
> +    }
>  }
>  
>  static void
> -- 
> 1.7.5.4
> 
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch