From: Jani Nikula <jani@nikula.org>
To: Austin Clements <amdragon@MIT.EDU>, notmuch@notmuchmail.org
Subject: Re: [PATCH v3 3/4] new: Merge error checks from add_files and
	add_files_recursive
In-Reply-To: <87vcjl5mba.fsf@nikula.org>
References: <1336429240-1114-1-git-send-email-amdragon@mit.edu>
	<1337887922-13163-1-git-send-email-amdragon@mit.edu>
	<1337887922-13163-4-git-send-email-amdragon@mit.edu>
	<87vcjl5mba.fsf@nikula.org>
User-Agent: Notmuch/0.13+13~gc259b9a (http://notmuchmail.org) Emacs/23.3.1
	(i686-pc-linux-gnu)
Date: Fri, 25 May 2012 00:08:55 +0300
Message-ID: <87sjep5lso.fsf@nikula.org>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Precedence: list


Grrh, sorry for the spam. My favourite MUA told me:

"Sending...failed to WARNING: gnome-keyring:: couldn't connect to:
/tmp/keyring-Ky8i5h/pkcs11: No such file or directory"

but sent the message anyway. Apparently msmpt tries to chat with the
gnome-keyring-daemon even though that should not happen per the man
page, and would be unnecessary and useless anyway.

J.


On Thu, 24 May 2012, Jani Nikula <jani@nikula.org> wrote:
> 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