* [PATCH] Solaris doesn't have 'struct dirent::d_type'
@ 2009-12-20 13:20 Tomas Carnecky
2009-12-20 18:02 ` James Westby
0 siblings, 1 reply; 3+ messages in thread
From: Tomas Carnecky @ 2009-12-20 13:20 UTC (permalink / raw)
To: notmuch
Use stat(2) instead.
Signed-off-by: Tomas Carnecky <tom@dbservice.com>
---
There is a second issue that prevents notmuch from working on Solaris:
the getpwuid_r() prototype doesn't have the last argument. But that can
be easily worked around by setting -D_POSIX_PTHREAD_SEMANTICS on the
compiler commandline. Do you want to use uname to detect the platform
and define platform-specific code or can I unconditionally add that
define to CFLAGS?
notmuch-new.c | 22 +++++++++++++++++-----
1 files changed, 17 insertions(+), 5 deletions(-)
diff --git a/notmuch-new.c b/notmuch-new.c
index 9d20616..837ae4f 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -90,12 +90,18 @@ static int ino_cmp(const struct dirent **a, const struct dirent **b)
* Return 1 if the directory looks like a Maildir and 0 otherwise.
*/
static int
-is_maildir (struct dirent **entries, int count)
+is_maildir (const char *path, struct dirent **entries, int count)
{
int i, found = 0;
for (i = 0; i < count; i++) {
- if (entries[i]->d_type != DT_DIR) continue;
+ char pbuf[PATH_MAX];
+ snprintf(pbuf, PATH_MAX, "%s/%s", path, entries[i]->d_name);
+
+ struct stat buf;
+ if (stat(pbuf, &buf) == -1 || !S_ISDIR(buf.st_mode))
+ continue;
+
if (strcmp(entries[i]->d_name, "new") == 0 ||
strcmp(entries[i]->d_name, "cur") == 0 ||
strcmp(entries[i]->d_name, "tmp") == 0)
@@ -178,7 +184,13 @@ add_files_recursive (notmuch_database_t *notmuch,
/* If this directory hasn't been modified since the last
* add_files, then we only need to look further for
* sub-directories. */
- if (path_mtime <= path_dbtime && entry->d_type == DT_REG)
+ struct stat buf;
+ char pbuf[PATH_MAX];
+ snprintf(pbuf, PATH_MAX, "%s/%s", path, entry->d_name);
+ if (stat(pbuf, &buf) == -1)
+ continue;
+
+ if (path_mtime <= path_dbtime && S_ISREG(buf.st_mode))
continue;
/* Ignore special directories to avoid infinite recursion.
@@ -188,9 +200,9 @@ add_files_recursive (notmuch_database_t *notmuch,
* user specify files to be ignored. */
if (strcmp (entry->d_name, ".") == 0 ||
strcmp (entry->d_name, "..") == 0 ||
- (entry->d_type == DT_DIR &&
+ (S_ISDIR(buf.st_mode) &&
(strcmp (entry->d_name, "tmp") == 0) &&
- is_maildir (namelist, num_entries)) ||
+ is_maildir (path, namelist, num_entries)) ||
strcmp (entry->d_name, ".notmuch") ==0)
{
continue;
--
1.6.6.rc1.39.g9a42
^ permalink raw reply related [flat|nested] 3+ messages in thread
* [PATCH] Solaris doesn't have 'struct dirent::d_type'
2009-12-20 13:20 [PATCH] Solaris doesn't have 'struct dirent::d_type' Tomas Carnecky
@ 2009-12-20 18:02 ` James Westby
2009-12-21 21:37 ` Tomas Carnecky
0 siblings, 1 reply; 3+ messages in thread
From: James Westby @ 2009-12-20 18:02 UTC (permalink / raw)
To: notmuch
From: Tomas Carnecky <tom@dbservice.com>
Use stat(2) instead.
Signed-off-by: Tomas Carnecky <tom@dbservice.com>
Signed-off-by: James Westby <jw+debian@jameswestby.net>
---
The original patch duplicated asprintf and stat calls, rearraging
the code means we don't need to.
I have a concern about the duplicated stats in is_maildir, but they
are not so easy to save. I ran a quick timing test (3931 files), dropping
caches before each set:
master:
real 2m3.545s
real 1m34.571s
real 1m36.005s
original patch:
real 2m18.114s
real 1m34.843s
real 1m36.317s
revised patch:
real 2m5.890s
real 1m36.387s
real 1m36.453s
This shoes there is little impact of the code, but given that it is
around one percent we may want to make it conditional on platform
and save the extra stat calls.
Thanks,
James
notmuch-new.c | 46 ++++++++++++++++++++++++++--------------------
1 files changed, 26 insertions(+), 20 deletions(-)
diff --git a/notmuch-new.c b/notmuch-new.c
index 9d20616..c6f4963 100644
--- a/notmuch-new.c
+++ b/notmuch-new.c
@@ -90,12 +90,18 @@ static int ino_cmp(const struct dirent **a, const struct dirent **b)
* Return 1 if the directory looks like a Maildir and 0 otherwise.
*/
static int
-is_maildir (struct dirent **entries, int count)
+is_maildir (const char *path, struct dirent **entries, int count)
{
int i, found = 0;
for (i = 0; i < count; i++) {
- if (entries[i]->d_type != DT_DIR) continue;
+ char pbuf[PATH_MAX];
+ snprintf(pbuf, PATH_MAX, "%s/%s", path, entries[i]->d_name);
+
+ struct stat buf;
+ if (stat(pbuf, &buf) == -1 || !S_ISDIR(buf.st_mode))
+ continue;
+
if (strcmp(entries[i]->d_name, "new") == 0 ||
strcmp(entries[i]->d_name, "cur") == 0 ||
strcmp(entries[i]->d_name, "tmp") == 0)
@@ -178,24 +184,6 @@ add_files_recursive (notmuch_database_t *notmuch,
/* If this directory hasn't been modified since the last
* add_files, then we only need to look further for
* sub-directories. */
- if (path_mtime <= path_dbtime && entry->d_type == DT_REG)
- continue;
-
- /* Ignore special directories to avoid infinite recursion.
- * Also ignore the .notmuch directory.
- */
- /* XXX: Eventually we'll want more sophistication to let the
- * user specify files to be ignored. */
- if (strcmp (entry->d_name, ".") == 0 ||
- strcmp (entry->d_name, "..") == 0 ||
- (entry->d_type == DT_DIR &&
- (strcmp (entry->d_name, "tmp") == 0) &&
- is_maildir (namelist, num_entries)) ||
- strcmp (entry->d_name, ".notmuch") ==0)
- {
- continue;
- }
-
next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name);
if (stat (next, st)) {
@@ -216,6 +204,24 @@ add_files_recursive (notmuch_database_t *notmuch,
goto DONE;
}
+ if (path_mtime <= path_dbtime && S_ISREG(st->st_mode))
+ continue;
+
+ /* Ignore special directories to avoid infinite recursion.
+ * Also ignore the .notmuch directory.
+ */
+ /* XXX: Eventually we'll want more sophistication to let the
+ * user specify files to be ignored. */
+ if (strcmp (entry->d_name, ".") == 0 ||
+ strcmp (entry->d_name, "..") == 0 ||
+ (S_ISDIR(st->st_mode) &&
+ (strcmp (entry->d_name, "tmp") == 0) &&
+ is_maildir (path, namelist, num_entries)) ||
+ strcmp (entry->d_name, ".notmuch") ==0)
+ {
+ continue;
+ }
+
if (S_ISREG (st->st_mode)) {
/* If the file hasn't been modified since the last
* add_files, then we need not look at it. */
--
1.6.3.3
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] Solaris doesn't have 'struct dirent::d_type'
2009-12-20 18:02 ` James Westby
@ 2009-12-21 21:37 ` Tomas Carnecky
0 siblings, 0 replies; 3+ messages in thread
From: Tomas Carnecky @ 2009-12-21 21:37 UTC (permalink / raw)
To: James Westby; +Cc: notmuch
On 12/20/09 7:02 PM, James Westby wrote:
> From: Tomas Carnecky<tom@dbservice.com>
>
> Use stat(2) instead.
>
> Signed-off-by: Tomas Carnecky<tom@dbservice.com>
> Signed-off-by: James Westby<jw+debian@jameswestby.net>
> ---
>
> The original patch duplicated asprintf and stat calls, rearraging
> the code means we don't need to.
>
> I have a concern about the duplicated stats in is_maildir, but they
> are not so easy to save. I ran a quick timing test (3931 files), dropping
> caches before each set:
>
> master:
> real 2m3.545s
> real 1m34.571s
> real 1m36.005s
>
> original patch:
> real 2m18.114s
> real 1m34.843s
> real 1m36.317s
>
> revised patch:
> real 2m5.890s
> real 1m36.387s
> real 1m36.453s
>
> This shoes there is little impact of the code, but given that it is
> around one percent we may want to make it conditional on platform
> and save the extra stat calls.
If performance regression is an issue, something like this could be used
to keep the current code paths in linux and stat() on other platforms:
static bool
is_dir(const char *path, struct dirent *dirent)
{
#if defined(__sun__)
... sprintf, stat etc
#else
(void) path;
return dirent->d_type == DT_DIR;
#endif
}
tom
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2009-12-21 22:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2009-12-20 13:20 [PATCH] Solaris doesn't have 'struct dirent::d_type' Tomas Carnecky
2009-12-20 18:02 ` James Westby
2009-12-21 21:37 ` Tomas Carnecky
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).