On Thu, 24 May 2012, Austin Clements <amdragon@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@notmuchmail.org > http://notmuchmail.org/mailman/listinfo/notmuch