[notmuch] [PATCH] d_type fix

[Carl Worth]

On Thu, 21 Jan 2010 15:10:18 -0500, Geo Carncross  wrote:
> A review of notmuch-new.c shows three uses of ->d_type:

Excellent, thank you.

> Near line 153, in _entries_resemble_maildir() we can simply allow for
> DT_UNKNOWN. This would fail if people have MH-style folders which have
> three folders called "new" "cur" and "tmp", but that seems unlikely, in
> which case the "tmp" folder would simply not be scanned.

The check will also fail if someone happens to have two regular files
named "cur" and "new" next to a directory named "tmp", (which would then
be ignored). But I agree this seems very unlikely.

> [snip]

The rest of your analysis looks good to me. And it's nice to see how
simple the final patch is.

> Attached is a patch which was tested with reiserfs. It should also work
> with xfs.

I had been waiting to apply the previous patch until I could test it on
a filesystem returning DT_UNKNOWN. But this one looks clean enough to
apply immediately.

One thing that would have made that easier would be a commit message,
(such as making a commit with "git commit" then using "git format-patch"
and "git send-email" to send it). But I'll go ahead and make one up.

Thanks again!

-Carl
-- next part --
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: not available
URL: 

Re: [notmuch] [PATCH] d_type fix

[Carl Worth]

On Thu, 21 Jan 2010 15:10:18 -0500, Geo Carncross 
 wrote:
> A review of notmuch-new.c shows three uses of ->d_type:

Excellent, thank you.

> Near line 153, in _entries_resemble_maildir() we can simply allow for
> DT_UNKNOWN. This would fail if people have MH-style folders which have
> three folders called "new" "cur" and "tmp", but that seems unlikely, in
> which case the "tmp" folder would simply not be scanned.

The check will also fail if someone happens to have two regular files
named "cur" and "new" next to a directory named "tmp", (which would then
be ignored). But I agree this seems very unlikely.

> [snip]

The rest of your analysis looks good to me. And it's nice to see how
simple the final patch is.

> Attached is a patch which was tested with reiserfs. It should also work
> with xfs.

I had been waiting to apply the previous patch until I could test it on
a filesystem returning DT_UNKNOWN. But this one looks clean enough to
apply immediately.

One thing that would have made that easier would be a commit message,
(such as making a commit with "git commit" then using "git format-patch"
and "git send-email" to send it). But I'll go ahead and make one up.

Thanks again!

-Carl


pgpew0jv21nyl.pgp
Description: PGP signature
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

[notmuch] [PATCH] d_type fix

[Geo Carncross]

A review of notmuch-new.c shows three uses of ->d_type:

Near line 153, in _entries_resemble_maildir() we can simply allow for
DT_UNKNOWN. This would fail if people have MH-style folders which have
three folders called "new" "cur" and "tmp", but that seems unlikely, in
which case the "tmp" folder would simply not be scanned.

Near line 273 in add_files_recursive() we have another check. If
DT_UNKNOWN, we fall through, then add_files_recursive() does a stat
almost immediately, returning with success if the path isn't a
directory.

Thus, the fallback is already written.

Finally, near line 343, in add_files_recursive() (a long function) we
have another check. Here we can simply treat DT_UNKNOWN as DT_LNK, since
the logic for the stat() results are the same.

Attached is a patch which was tested with reiserfs. It should also work
with xfs.


-- next part --
A non-text attachment was scrubbed...
Name: notmuch_new_d_type_fix.patch
Type: text/x-patch
Size: 1144 bytes
Desc: not available
URL: 

[notmuch] [PATCH] d_type fix

[Geo Carncross]

A review of notmuch-new.c shows three uses of ->d_type:

Near line 153, in _entries_resemble_maildir() we can simply allow for
DT_UNKNOWN. This would fail if people have MH-style folders which have
three folders called "new" "cur" and "tmp", but that seems unlikely, in
which case the "tmp" folder would simply not be scanned.

Near line 273 in add_files_recursive() we have another check. If
DT_UNKNOWN, we fall through, then add_files_recursive() does a stat
almost immediately, returning with success if the path isn't a
directory.

Thus, the fallback is already written.

Finally, near line 343, in add_files_recursive() (a long function) we
have another check. Here we can simply treat DT_UNKNOWN as DT_LNK, since
the logic for the stat() results are the same.

Attached is a patch which was tested with reiserfs. It should also work
with xfs.


diff --git a/notmuch-new.c b/notmuch-new.c
index b740ee2..3e6b96a 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -153,7 +153,7 @@ _entries_resemble_maildir (struct dirent **entries, int count)
 int i, found = 0;
 
 for (i = 0; i < count; i++) {
-	if (entries[i]->d_type != DT_DIR)
+	if (entries[i]->d_type != DT_DIR && entries[i]->d_type != DT_UNKNOWN)
 	continue;
 
 	if (strcmp(entries[i]->d_name, "new") == 0 ||
@@ -273,7 +273,8 @@ add_files_recursive (notmuch_database_t *notmuch,
 
 	entry = fs_entries[i];
 
-	if (entry->d_type != DT_DIR && entry->d_type != DT_LNK)
+	if (entry->d_type != DT_DIR && entry->d_type != DT_LNK
+			&& entry->d_type != DT_UNKNOWN)
 	continue;
 
 	/* Ignore special directories to avoid infinite recursion.
@@ -343,7 +344,7 @@ add_files_recursive (notmuch_database_t *notmuch,
 
 	/* If we're looking at a symlink, we only want to add it if it
 	 * links to a regular file, (and not to a directory, say). */
-	if (entry->d_type == DT_LNK) {
+	if (entry->d_type == DT_LNK || entry->d_type == DT_UNKNOWN) {
 	int err;
 
 	next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch