Re: [PATCH] Various small clean-ups to doc ID set code.

diff --git a/48/f5678fabeb32dd72dbefed16e672b3867da1e6 b/48/f5678fabeb32dd72dbefed16e672b3867da1e6
new file mode 100644 (file)
index 0000000..e5e42ea
--- /dev/null
+++ b/48/f5678fabeb32dd72dbefed16e672b3867da1e6
@@ -0,0 +1,110 @@
+Return-Path: <cworth@cworth.org>\r
+X-Original-To: notmuch@notmuchmail.org\r
+Delivered-To: notmuch@notmuchmail.org\r
+Received: from localhost (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id 970CB429E22\r
+       for <notmuch@notmuchmail.org>; Fri, 28 Jan 2011 14:02:53 -0800 (PST)\r
+X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
+X-Spam-Flag: NO\r
+X-Spam-Score: -0.99\r
+X-Spam-Level: \r
+X-Spam-Status: No, score=-0.99 tagged_above=-999 required=5\r
+       tests=[ALL_TRUSTED=-1, T_MIME_NO_TEXT=0.01] autolearn=disabled\r
+Received: from olra.theworths.org ([127.0.0.1])\r
+       by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
+       with ESMTP id NHVzgcad1a9x; Fri, 28 Jan 2011 14:02:52 -0800 (PST)\r
+Received: from yoom.home.cworth.org (localhost [127.0.0.1])\r
+       by olra.theworths.org (Postfix) with ESMTP id EAA99429E20;\r
+       Fri, 28 Jan 2011 14:02:50 -0800 (PST)\r
+Received: by yoom.home.cworth.org (Postfix, from userid 1000)\r
+       id ED09925449D; Sat, 29 Jan 2011 07:36:13 +1000 (EST)\r
+From: Carl Worth <cworth@cworth.org>\r
+To: Austin Clements <amdragon@MIT.EDU>\r
+Subject: Re: [PATCH] Various small clean-ups to doc ID set code.\r
+In-Reply-To: <20101208220153.GT2447@mit.edu>\r
+References: <20101117192826.GU2439@mit.edu>\r
+       <874oap5aek.fsf@yoom.home.cworth.org>\r
+       <20101208215844.GS2447@mit.edu> <20101208220153.GT2447@mit.edu>\r
+User-Agent: Notmuch/0.5 (http://notmuchmail.org) Emacs/23.2.1\r
+       (i486-pc-linux-gnu)\r
+Date: Sat, 29 Jan 2011 07:36:13 +1000\r
+Message-ID: <8762t8n22a.fsf@yoom.home.cworth.org>\r
+MIME-Version: 1.0\r
+Content-Type: multipart/signed; boundary="=-=-=";\r
+       micalg=pgp-sha1; protocol="application/pgp-signature"\r
+Cc: notmuch@notmuchmail.org\r
+X-BeenThere: notmuch@notmuchmail.org\r
+X-Mailman-Version: 2.1.13\r
+Precedence: list\r
+List-Id: "Use and development of the notmuch mail system."\r
+       <notmuch.notmuchmail.org>\r
+List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
+List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
+List-Post: <mailto:notmuch@notmuchmail.org>\r
+List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
+List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
+       <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
+X-List-Received-Date: Fri, 28 Jan 2011 22:02:54 -0000\r
+\r
+--=-=-=\r
+Content-Transfer-Encoding: quoted-printable\r
+\r
+On Wed, 8 Dec 2010 17:01:53 -0500, Austin Clements <amdragon@MIT.EDU> wrote:\r
+> Remove the repeated "sizeof (doc_ids->bitmap[0])" that bothered cworth\r
+> by instead defining macros to compute the word and bit offset of a\r
+> given bit in the bitmap.\r
+>=20\r
+> Don't require the caller of _notmuch_doc_id_set_init to pass in a\r
+> correct bound; instead compute it from the array.  This simplifies the\r
+> caller and makes this interface easier to use correctly.\r
+...\r
+> +#define BITMAP_WORD(bit) ((bit) / sizeof (unsigned int))\r
+> +#define BITMAP_BIT(bit) ((bit) % sizeof (unsigned int))\r
+\r
+These macros look great, they definitely simplify the code.\r
+\r
+>  _notmuch_doc_id_set_init (void *ctx,\r
+>                        notmuch_doc_id_set_t *doc_ids,\r
+> -                      GArray *arr, unsigned int bound)\r
+> +                      GArray *arr)\r
+...\r
+> +    for (unsigned int i =3D 0; i < arr->len; i++)\r
+> +    max =3D MAX(max, g_array_index (arr, unsigned int, i));\r
+\r
+And computing an argument automatically definitely makes the interface\r
+easier to use. So that's good too. But those two changes are independent\r
+so really need to be in separate commits.\r
+\r
+> -    if (doc_id >=3D doc_ids->bound)\r
+> +    if (doc_id > doc_ids->max)\r
+\r
+And this looks really like a *third* independent change to me.\r
+\r
+A code change like the above has the chance to introduce (or fix) an\r
+off-by-one bug---or even leave the code effectively unchanged as the\r
+intent is here.\r
+\r
+In order to distinguish all of those cases, I'd like to see a change\r
+like this as a minimal change, and described in the commit\r
+message. (Rather than hidden amongst "various cleanups" that are mostly\r
+about replacing some common code with a macro.)\r
+\r
+So I'd be happy to see this patch broken up and sent again.\r
+\r
+=2DCarl\r
+\r
+=2D-=20\r
+carl.d.worth@intel.com\r
+\r
+--=-=-=\r
+Content-Type: application/pgp-signature\r
+\r
+-----BEGIN PGP SIGNATURE-----\r
+Version: GnuPG v1.4.10 (GNU/Linux)\r
+\r
+iD8DBQFNQzbN6JDdNq8qSWgRAvgBAJ42F8FC3jWC+TP6LrTIHj/aihdZYACeLuNL\r
+dZ7fhl3lzHBTsU6u56fBAJ4=\r
+=z68Z\r
+-----END PGP SIGNATURE-----\r
+--=-=-=--\r