Return-Path: X-Original-To: notmuch@notmuchmail.org Delivered-To: notmuch@notmuchmail.org Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id 199B4431FAF for ; Wed, 11 Apr 2012 12:36:14 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -0.7 X-Spam-Level: X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled Received: from olra.theworths.org ([127.0.0.1]) by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024) with ESMTP id 0yjZUp1Ugt2q for ; Wed, 11 Apr 2012 12:36:13 -0700 (PDT) Received: from dmz-mailsec-scanner-8.mit.edu (DMZ-MAILSEC-SCANNER-8.MIT.EDU [18.7.68.37]) by olra.theworths.org (Postfix) with ESMTP id 3F5CC431FAE for ; Wed, 11 Apr 2012 12:36:13 -0700 (PDT) X-AuditID: 12074425-b7f4a6d0000008e0-04-4f85dd2c59a8 Received: from mailhub-auth-2.mit.edu ( [18.7.62.36]) by dmz-mailsec-scanner-8.mit.edu (Symantec Messaging Gateway) with SMTP id A1.33.02272.C2DD58F4; Wed, 11 Apr 2012 15:36:12 -0400 (EDT) Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103]) by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id q3BJaB68025613; Wed, 11 Apr 2012 15:36:11 -0400 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91]) (authenticated bits=0) (User authenticated as amdragon@ATHENA.MIT.EDU) by outgoing.mit.edu (8.13.6/8.12.4) with ESMTP id q3BJa97I025836 (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT); Wed, 11 Apr 2012 15:36:10 -0400 (EDT) Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77) (envelope-from ) id 1SI3Kr-0000q7-Kj; Wed, 11 Apr 2012 15:36:09 -0400 Date: Wed, 11 Apr 2012 15:36:09 -0400 From: Austin Clements To: Vladimir.Marek@oracle.com Subject: Re: [PATCH 2/4] dirent->d_type not available on Soalris Message-ID: <20120411193609.GC13549@mit.edu> References: <1333989127-21523-1-git-send-email-Vladimir.Marek@oracle.com> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii Content-Disposition: inline In-Reply-To: <1333989127-21523-1-git-send-email-Vladimir.Marek@oracle.com> User-Agent: Mutt/1.5.21 (2010-09-15) X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFprFKsWRmVeSWpSXmKPExsUixG6noqtzt9XfoPONicX1mzOZLTpu72az mHF+F4sDs8ezVbeYPT4+vcXisezoT8YA5igum5TUnMyy1CJ9uwSujKtz1jEVTFCqmLTrAXMD 4zGpLkZODgkBE4lzk3YzQdhiEhfurWfrYuTiEBLYxyhxdf1bJghnA6PEr/7JjBDOSSaJee2f WCGcJYwSf//cBirj4GARUJWYdUwFZBSbgIbEtv3LGUFsEQFZif9TPoGtYBawk5j24hiYLSzg IPHv0Sx2EJtXQEfixcdHUNsmMEqcPnQbKiEocXLmExaIZi2JG/9egu1iFpCWWP6PAyTMKeAl 8WX7cjYQW1RARWLKyW1sExiFZiHpnoWkexZC9wJG5lWMsim5Vbq5iZk5xanJusXJiXl5qUW6 Fnq5mSV6qSmlmxjBwe6iuoNxwiGlQ4wCHIxKPLy7JrT6C7EmlhVX5h5ilORgUhLlXXcVKMSX lJ9SmZFYnBFfVJqTWnyIUYKDWUmE12V2i78Qb0piZVVqUT5MSpqDRUmcV1PrnZ+QQHpiSWp2 ampBahFMVoaDQ0mC98QdoKGCRanpqRVpmTklCGkmDk6Q4TxAw5eD1PAWFyTmFmemQ+RPMepy PD/Ue4VRiCUvPy9VSpx3OkiRAEhRRmke3BxYknrFKA70ljDvCpAqHmCCg5v0CmgJE9CSz5NB PiguSURISTUwng9XyfqRwr/u1qE7n+8sr7v0Zq/SwkUBkjqZ8YrlD5Xmd7/jdvtUvtxnbvt3 95ORvDdvTFixWJrJeWdCnejMjnDGBfs/bL3EtDRuLZfQbIeUr/+zvTT+F3uuYl/IV8fdlma4 sGKq9IW28MIbDD1friudDVmlk78v13nm9S/24vcLmjzEl+8+rMRSnJFoqMVcVJwIADzy/Iwt AwAA Cc: notmuch@notmuchmail.org, Vladimir Marek X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.13 Precedence: list List-Id: "Use and development of the notmuch mail system." List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , X-List-Received-Date: Wed, 11 Apr 2012 19:36:14 -0000 Correct me if I'm mistaken, but d_name will only be a basename, so your calls to stat will fail for files that are not in the current directory. I think in all of the situations you had to call stat, we already construct the absolute path of the file (sometimes a little later in the code, but it can easily be moved), so this should be easy to fix. Rather than sprinkling portability code throughout notmuch-new, it seems like it would be simpler if this logic were wrapped in a separate function. For example, something along the (completely untested) lines of, static mode_t dirent_type (const struct *entry, const char *abspath) { struct stat statbuf; #ifdef _DIRENT_HAVE_D_TYPE static const mode_t modes[] = { [DT_BLK] = S_IFBLK, [DT_CHR] = S_IFCHR, [DT_DIR] = S_IFDIR, [DT_FIFO] = S_IFIFO, [DT_LNK] = S_IFLNK, [DT_REG] = S_IFREG, [DT_SOCK] = S_IFSOCK }; if (entry->d_type >= 0 && entry->d_type < sizeof(modes)/sizeof(modes[0]) && modes[entry->d_type]) return modes[entry->d_type]; #endif if (stat(abspath, &statbuf) == -1) return -1; return statbuf.st_mode & S_IFMT; } This has the added benefit of correctly handling DT_UNKNOWN, which we currently don't. Instead of taking the absolute path of the file, this could take the absolute path of the containing directory and construct the full path from that and d_name; that would probably be a nicer interface, but it would be redundant computation. Quoth Vladimir.Marek@oracle.com on Apr 09 at 6:32 pm: > From: Vladimir Marek > > The inspiration was taken from similar issue in mutt: > http://does-not-exist.org/mail-archives/mutt-dev/msg11290.html > > Signed-off-by: Vladimir Marek > --- > notmuch-new.c | 28 ++++++++++++++++++++++++++++ > 1 files changed, 28 insertions(+), 0 deletions(-) > > diff --git a/notmuch-new.c b/notmuch-new.c > index 4f13535..3d265bd 100644 > --- a/notmuch-new.c > +++ b/notmuch-new.c > @@ -21,6 +21,9 @@ > #include "notmuch-client.h" > > #include > +#ifndef _DIRENT_HAVE_D_TYPE > +#include > +#endif > > typedef struct _filename_node { > char *filename; > @@ -167,7 +170,14 @@ _entries_resemble_maildir (struct dirent **entries, int count) > int i, found = 0; > > for (i = 0; i < count; i++) { > +#ifdef _DIRENT_HAVE_D_TYPE > if (entries[i]->d_type != DT_DIR && entries[i]->d_type != DT_UNKNOWN) > +#else > + struct stat statbuf; > + if (stat(entries[i]->d_name, &statbuf) == -1) > + continue; > + if (! S_ISDIR(statbuf.st_mode)) > +#endif > continue; > > if (strcmp(entries[i]->d_name, "new") == 0 || > @@ -258,6 +268,9 @@ add_files_recursive (notmuch_database_t *notmuch, > struct stat st; > notmuch_bool_t is_maildir, new_directory; > const char **tag; > +#ifndef _DIRENT_HAVE_D_TYPE > + struct stat statbuf; > +#endif > > if (stat (path, &st)) { > fprintf (stderr, "Error reading directory %s: %s\n", > @@ -328,9 +341,16 @@ add_files_recursive (notmuch_database_t *notmuch, > * scandir results, then it might be a directory (and if not, > * then we'll stat and return immediately in the next level of > * recursion). */ > +#ifdef _DIRENT_HAVE_D_TYPE > if (entry->d_type != DT_DIR && > entry->d_type != DT_LNK && > entry->d_type != DT_UNKNOWN) > +#else > + if (stat(entry->d_name, &statbuf) == -1) > + continue; > + if (!(statbuf.st_mode & S_IFDIR) && > + !(statbuf.st_mode & S_IFLNK)) > +#endif > { > continue; > } > @@ -427,7 +447,11 @@ add_files_recursive (notmuch_database_t *notmuch, > * > * In either case, a stat does the trick. > */ > +#ifdef _DIRENT_HAVE_D_TYPE > if (entry->d_type == DT_LNK || entry->d_type == DT_UNKNOWN) { > +#else > + if (stat(entry->d_name, &statbuf) == -1 || statbuf.st_mode & S_IFLNK) { > +#endif > int err; > > next = talloc_asprintf (notmuch, "%s/%s", path, entry->d_name); > @@ -443,7 +467,11 @@ add_files_recursive (notmuch_database_t *notmuch, > > if (! S_ISREG (st.st_mode)) > continue; > +#ifdef _DIRENT_HAVE_D_TYPE > } else if (entry->d_type != DT_REG) { > +#else > + } else if (statbuf.st_mode & S_IFREG) { > +#endif > continue; > } >