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 80B39431FBD; Mon, 21 Dec 2009 10:43:17 -0800 (PST) X-Virus-Scanned: Debian amavisd-new at olra.theworths.org 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 3JKw+NUQCsJG; Mon, 21 Dec 2009 10:43:16 -0800 (PST) Received: from yoom.home.cworth.org (localhost [127.0.0.1]) by olra.theworths.org (Postfix) with ESMTP id D1D43431FAE; Mon, 21 Dec 2009 10:43:16 -0800 (PST) Received: by yoom.home.cworth.org (Postfix, from userid 1000) id 889A8254306; Mon, 21 Dec 2009 10:43:16 -0800 (PST) From: Carl Worth To: James Westby , notmuch@notmuchmail.org In-Reply-To: <1261340852-20183-1-git-send-email-jw+debian@jameswestby.net> References: <871virzzjy.fsf@yoom.home.cworth.org> <1261340852-20183-1-git-send-email-jw+debian@jameswestby.net> Date: Mon, 21 Dec 2009 10:43:16 -0800 Message-ID: <87tyvknp6j.fsf@yoom.home.cworth.org> MIME-Version: 1.0 Content-Type: multipart/signed; boundary="=-=-="; micalg=pgp-sha1; protocol="application/pgp-signature" Subject: Re: [notmuch] [PATCH] Store documents for message-ids we haven't seen X-BeenThere: notmuch@notmuchmail.org X-Mailman-Version: 2.1.12 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: Mon, 21 Dec 2009 18:43:17 -0000 --=-=-= On Sun, 20 Dec 2009 20:27:32 +0000, James Westby wrote: > One case that isn't handled is when we don't find the thread > id of the parent, but then find the message itself. I believe > this case shouldn't happen, but you never know. It really shouldn't happen since we are holding a write lock on the database, (so there's no possible race condition here with another client delivering the parent message). But since you almost can't help but detect the case, (just noticing a NOTMUCH_STATUS_SUCCESS value from _create_for_message_id), please put an INTERNAL_ERROR there rather than marching along with an incorrect thread ID. > + // We have yet to see the referenced message, generate a thread id > + // for it if needed and store a dummy message for the parent. If we > + // find the mail later we will replace the dummy. Call me old-fashioned if you will, but I'd much rather have C style multi-line comments (/* ... */) rather than these C++-style comments with //. > + if (private_status == NOTMUCH_PRIVATE_STATUS_NO_DOCUMENT_FOUND) { > + // expected And I think this comment deserves a complete sentence before the condition. Something like: /* We expect this call to create a new document (return NO_DOCUMENT_FOUND) */ -Carl --=-=-= Content-Type: application/pgp-signature -----BEGIN PGP SIGNATURE----- Version: GnuPG v1.4.10 (GNU/Linux) iD8DBQFLL8HE6JDdNq8qSWgRApiyAJoD4RgwIvKAw76GZsXQXQtjSJAnswCgnAHd NDrxx6eQxvZ9QVLjgVLq2ag= =NK5e -----END PGP SIGNATURE----- --=-=-=--