From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: from localhost (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id AB503431FBC for ; Sat, 23 Aug 2014 17:59:00 -0700 (PDT) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org X-Spam-Flag: NO X-Spam-Score: -2.3 X-Spam-Level: X-Spam-Status: No, score=-2.3 tagged_above=-999 required=5 tests=[RCVD_IN_DNSWL_MED=-2.3] 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 2Hr1+1xpMa5l for ; Sat, 23 Aug 2014 17:58:53 -0700 (PDT) Received: from dmz-mailsec-scanner-1.mit.edu (dmz-mailsec-scanner-1.mit.edu [18.9.25.12]) (using TLSv1 with cipher DHE-RSA-AES256-SHA (256/256 bits)) (No client certificate requested) by olra.theworths.org (Postfix) with ESMTPS id 84C23431FB6 for ; Sat, 23 Aug 2014 17:58:53 -0700 (PDT) X-AuditID: 1209190c-f795e6d000006c66-ef-53f938cc5799 Received: from mailhub-auth-2.mit.edu ( [18.7.62.36]) (using TLS with cipher AES256-SHA (256/256 bits)) (Client did not present a certificate) by dmz-mailsec-scanner-1.mit.edu (Symantec Messaging Gateway) with SMTP id 94.84.27750.CC839F35; Sat, 23 Aug 2014 20:58:52 -0400 (EDT) Received: from outgoing.mit.edu (outgoing-auth-1.mit.edu [18.9.28.11]) by mailhub-auth-2.mit.edu (8.13.8/8.9.2) with ESMTP id s7O0wqZq009560; Sat, 23 Aug 2014 20:58:52 -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.8/8.12.4) with ESMTP id s7O0woqS027618 (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT); Sat, 23 Aug 2014 20:58:51 -0400 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80) (envelope-from ) id 1XLM8w-0001tA-HW; Sat, 23 Aug 2014 20:58:50 -0400 From: Austin Clements To: David Bremner , notmuch@notmuchmail.org Subject: Re: [PATCH v3 04/13] lib: Database version 3: Introduce fine-grained "features" In-Reply-To: <87ppfqsv8s.fsf@maritornes.cs.unb.ca> References: <1406859003-11561-1-git-send-email-amdragon@mit.edu> <1406859003-11561-5-git-send-email-amdragon@mit.edu> <87ppfqsv8s.fsf@maritornes.cs.unb.ca> User-Agent: Notmuch/0.18.1+76~g58c9570 (http://notmuchmail.org) Emacs/23.4.1 (i486-pc-linux-gnu) Date: Sat, 23 Aug 2014 20:58:50 -0400 Message-ID: <87fvgmg0tx.fsf@awakening.csail.mit.edu> MIME-Version: 1.0 Content-Type: text/plain; charset=us-ascii X-Brightmail-Tracker: H4sIAAAAAAAAA+NgFnrAIsWRmVeSWpSXmKPExsUixG6nonvG4mewwdJflhY3WrsZLa7fnMns wOTxbNUtZo8th94zBzBFcdmkpOZklqUW6dslcGXMnfybueC3cMX0Ca8ZGxif83cxcnJICJhI LPv7gQXCFpO4cG89WxcjF4eQwGwmiU0r1rBAOBsZJZbfvscOUiUkcJpJ4sQKLojEEkaJ3lMN YAk2AQ2J37cWM4HYIgJ2Ej0XzoPFhQUiJOZ83wO2glPASOLOsmmsEINmM0q07UoFsUUFEiRO 9h9g7GLk4GARUJXo/c4FEuYFuu7t0jZ2CFtQ4uTMJ2BjmAW0JG78e8k0gVFgFpLULCSpBYxM qxhlU3KrdHMTM3OKU5N1i5MT8/JSi3QN9XIzS/RSU0o3MYICklOSZwfjm4NKhxgFOBiVeHg/ XPwRLMSaWFZcmXuIUZKDSUmU95XGz2AhvqT8lMqMxOKM+KLSnNTiQ4wSHMxKIrzFZkA53pTE yqrUonyYlDQHi5I471trq2AhgfTEktTs1NSC1CKYrAwHh5IE70xzoEbBotT01Iq0zJwShDQT ByfIcB6g4Y9BaniLCxJzizPTIfKnGBWlxHk3giQEQBIZpXlwvbCE8YpRHOgVYd6VIFU8wGQD 1/0KaDAT0ODpM76CDC5JREhJNTDanjqs9WKrRoZorE7+kmNHBfintS9/HbN3Ildo5raZlzfI PmJT75z0fhOz8QXu9PJ3N2oVF5bNOrCoW0zs0uTpKz/sFZKZ0N/0T9Hi2/pXy1NyjB7/W7Ri /vfpC9bcrPr46+aXP5win9bsZTWPTLvy6tER6Zt3XzfcWVraFSF48cuxTde/nPc+ZaHEUpyR aKjFXFScCAB2WsD38wIAAA== 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: Sun, 24 Aug 2014 00:59:00 -0000 On Sat, 23 Aug 2014, David Bremner wrote: > Austin Clements writes: >> >> + /* Bit mask of features used by this database. Features are >> + * named, independent aspects of the database schema. This is a >> + * bitwise-OR of NOTMUCH_FEATURE_* values (below). */ >> + unsigned int features; > > Should we be using a fixed size integer (uint_32t or whatever) for > features? iirc the metadata in the database is actually a string, so I > guess arbitrary precision there. Right; this doesn't matter for the on-disk format because these don't appear on disk. But you're right that in principle we could overflow this, leading to subtle bugs. I moved the enum above struct _notmuch_database, gave it a name and bitwise operators for C++, and used that enum name everywhere, so precision should never be a problem. >> +/* Bit masks for _notmuch_database::features. */ >> +enum { >> + /* If set, file names are stored in "file-direntry" terms. If >> + * unset, file names are stored in document data. >> + * >> + * Introduced: version 1. Implementation support: both for read; >> + * required for write. */ >> + NOTMUCH_FEATURE_FILE_TERMS = 1 << 0, > > I agree with Jani that the Implementation support: part is a bit > mystifying without the commit message. Maybe part of the commit message > could migrate here? Or maybe just add a pointer to the comment in database.cc. I stripped these out because I don't think they're maintainable. See my reply to Jani. >> + if (! *incompat_out) > > Should we support passing NULL for incompat_out? or at least check for > it? Added a guard so it's safe to pass NULL. >> @@ -1048,7 +1164,8 @@ notmuch_database_get_version (notmuch_database_t *notmuch) >> notmuch_bool_t >> notmuch_database_needs_upgrade (notmuch_database_t *notmuch) >> { >> - return notmuch->needs_upgrade; >> + return notmuch->mode == NOTMUCH_DATABASE_MODE_READ_WRITE && >> + (NOTMUCH_FEATURES_CURRENT & ~notmuch->features); >> } > > Maybe I'm not thinking hard enough here, but how does this deal with a > feature that is needed to open a database in read only mode? Maybe it > needs a comment for people not as clever as Austin ;). I'm not quite sure what you mean. notmuch_database_needs_upgrade returns false for read-only databases because you can't upgrade a read-only database. This was true before this patch, too, though it was less obvious. (Maybe that's not what you're asking?)