[PATCH v3 3/4] new: Merge error checks from add_files and add_files_recursive

[Jani Nikula]

On Thu, 24 May 2012, Austin Clements <amdragon at MIT.EDU> wrote:
> Before XXX, add_files_recursive could have been called on a symlink to
> a non-directory.  Hence, calling it on a non-directory was not an
> error, so a separate function, add_files, existed to fail loudly in
> situations where the path had to be a directory.

"Before XXX"?

Otherwise, this 3/4 and following 4/4 patch LGTM. I didn't bother
looking at 1/4 and 2/4 again, as you say there were no changes.


BR,
Jani.


>
> With the new stat-ing logic, add_files_recursive is always called on
> directories, so the separation of this logic is no longer necessary.
> Hence, this patch moves the strict error checking previously done by
> add_files into add_files_recursive.
> ---
>  notmuch-new.c |   27 +++++----------------------
>  1 file changed, 5 insertions(+), 22 deletions(-)
>
> diff --git a/notmuch-new.c b/notmuch-new.c
> index c64f1a7..2b05605 100644
> --- a/notmuch-new.c
> +++ b/notmuch-new.c
> @@ -308,11 +308,10 @@ add_files_recursive (notmuch_database_t *notmuch,
>      }
>      stat_time = time (NULL);
>  
> -    /* This is not an error since we may have recursed based on a
> -     * symlink to a regular file, not a directory, and we don't know
> -     * that until this stat. */
> -    if (! S_ISDIR (st.st_mode))
> -	return NOTMUCH_STATUS_SUCCESS;
> +    if (! S_ISDIR (st.st_mode)) {
> +	fprintf (stderr, "Error: %s is not a directory.\n", path);
> +	return NOTMUCH_STATUS_FILE_ERROR;
> +    }
>  
>      fs_mtime = st.st_mtime;
>  
> @@ -655,23 +654,7 @@ add_files (notmuch_database_t *notmuch,
>  	   const char *path,
>  	   add_files_state_t *state)
>  {
> -    notmuch_status_t status;
> -    struct stat st;
> -
> -    if (stat (path, &st)) {
> -	fprintf (stderr, "Error reading directory %s: %s\n",
> -		 path, strerror (errno));
> -	return NOTMUCH_STATUS_FILE_ERROR;
> -    }
> -
> -    if (! S_ISDIR (st.st_mode)) {
> -	fprintf (stderr, "Error: %s is not a directory.\n", path);
> -	return NOTMUCH_STATUS_FILE_ERROR;
> -    }
> -
> -    status = add_files_recursive (notmuch, path, state);
> -
> -    return status;
> +    return add_files_recursive (notmuch, path, state);
>  }
>  
>  /* XXX: This should be merged with the add_files function since it
> -- 
> 1.7.10
>
> _______________________________________________
> notmuch mailing list
> notmuch at notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch