78/5c3917617482b3e376918327740e90afc47827

   1 Return-Path: <amdragon@mit.edu>\r
   2 X-Original-To: notmuch@notmuchmail.org\r
   3 Delivered-To: notmuch@notmuchmail.org\r
   4 Received: from localhost (localhost [127.0.0.1])\r
   5         by olra.theworths.org (Postfix) with ESMTP id 19683431FB6\r
   6         for <notmuch@notmuchmail.org>; Sun,  3 Feb 2013 12:01:34 -0800 (PST)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: -0.7\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.7 tagged_above=-999 required=5\r
  12         tests=[RCVD_IN_DNSWL_LOW=-0.7] autolearn=disabled\r
  13 Received: from olra.theworths.org ([127.0.0.1])\r
  14         by localhost (olra.theworths.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15         with ESMTP id 3Bx+QfTL8Dd3 for <notmuch@notmuchmail.org>;\r
  16         Sun,  3 Feb 2013 12:01:32 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-3.mit.edu (DMZ-MAILSEC-SCANNER-3.MIT.EDU\r
  18         [18.9.25.14])\r
  19         by olra.theworths.org (Postfix) with ESMTP id 17843431FAE\r
  20         for <notmuch@notmuchmail.org>; Sun,  3 Feb 2013 12:01:32 -0800 (PST)\r
  21 X-AuditID: 1209190e-b7f266d0000008cb-e9-510ec21b3dbc\r
  22 Received: from mailhub-auth-4.mit.edu ( [18.7.62.39])\r
  23         by dmz-mailsec-scanner-3.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id 99.3E.02251.B12CE015; Sun,  3 Feb 2013 15:01:31 -0500 (EST)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH-1.MIT.EDU [18.9.28.11])\r
  26         by mailhub-auth-4.mit.edu (8.13.8/8.9.2) with ESMTP id r13K1UlO014252; \r
  27         Sun, 3 Feb 2013 15:01:30 -0500\r
  28 Received: from awakening.csail.mit.edu (awakening.csail.mit.edu [18.26.4.91])\r
  29         (authenticated bits=0)\r
  30         (User authenticated as amdragon@ATHENA.MIT.EDU)\r
  31         by outgoing.mit.edu (8.13.8/8.12.4) with ESMTP id r13K1R7f024955\r
  32         (version=TLSv1/SSLv3 cipher=DHE-RSA-AES128-SHA bits=128 verify=NOT);\r
  33         Sun, 3 Feb 2013 15:01:29 -0500\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.80)\r
  35         (envelope-from <amdragon@mit.edu>)\r
  36         id 1U25kl-0004uM-Hj; Sun, 03 Feb 2013 15:01:27 -0500\r
  37 Date: Sun, 3 Feb 2013 15:01:27 -0500\r
  38 From: Austin Clements <amdragon@MIT.EDU>\r
  39 To: Robert Mast <beheerder@tekenbeetziekten.nl>\r
  40 Subject: Re: [PATCH] This patch is a little finger excercise for working with\r
  41         git. I found a piece of code that I didn't understand at first.\r
  42 Message-ID: <20130203200127.GM4445@mit.edu>\r
  43 References: <1359917491-17178-1-git-send-email-beheerder@tekenbeetziekten.nl>\r
  44 MIME-Version: 1.0\r
  45 Content-Type: text/plain; charset=us-ascii\r
  46 Content-Disposition: inline\r
  47 In-Reply-To: <1359917491-17178-1-git-send-email-beheerder@tekenbeetziekten.nl>\r
  48 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  49 X-Brightmail-Tracker:\r
  50  H4sIAAAAAAAAA+NgFprAKsWRmVeSWpSXmKPExsUixG6nrit9iC/Q4PVxEYuTHe4W12/OZHZg\r
  51         8ni26hazx5mDzxkDmKK4bFJSczLLUov07RK4Mvb/38pc8FyoYsnd9SwNjG/4uhg5OSQETCR2\r
  52         /W9hh7DFJC7cW8/WxcjFISSwj1Fi1vwrrBDOBkaJg/2HmSCcU0wS6z+ehCpbwijxuvM4C0g/\r
  53         i4CKxJ5Xu8FsNgENiW37lzOC2CIC+hJbT3xlBrGZBaQlvv1uBpskLNDPKHGv8wwTSIJXQFvi\r
  54         x+NnYEVCAr4S2x+3s0HEBSVOznzCAtGsJXHj30ugeg6wQcv/cYCEOQX8JB4fecYKYosC3TDl\r
  55         5Da2CYxCs5B0z0LSPQuhewEj8ypG2ZTcKt3cxMyc4tRk3eLkxLy81CJdY73czBK91JTSTYyg\r
  56         wOaU5NvB+PWg0iFGAQ5GJR7eCdt4A4VYE8uKK3MPMUpyMCmJ8noc4AsU4kvKT6nMSCzOiC8q\r
  57         zUktPsQowcGsJMJ71Qgox5uSWFmVWpQPk5LmYFES572SctNfSCA9sSQ1OzW1ILUIJivDwaEk\r
  58         wct9EKhRsCg1PbUiLTOnBCHNxMEJMpwHaLg8SA1vcUFibnFmOkT+FKMux64JHc8ZhVjy8vNS\r
  59         pcR5v4BcJwBSlFGaBzcHlpBeMYoDvSXM+xKkigeYzOAmvQJawgS0pK2SG2RJSSJCSqqBMSNl\r
  60         j6Dh84eHFacyMbLf4Xz1X+/Ulx6nhSGfptY+1fO9sMPy/HrznbkPvwQFNdkK8NjNPrOQheV9\r
  61         V4hrbHRZBm+7MYtJxbHgaw93zP+eGVe0ITNdf2625bHJPMvtDkU/W3hEINRXs9LAaZ78w8kz\r
  62         N/wzuHratWmvrv6SaveqxjV+jnFLrzRLKLEUZyQaajEXFScCABCaC8sjAwAA\r
  63 Cc: notmuch@notmuchmail.org\r
  64 X-BeenThere: notmuch@notmuchmail.org\r
  65 X-Mailman-Version: 2.1.13\r
  66 Precedence: list\r
  67 List-Id: "Use and development of the notmuch mail system."\r
  68         <notmuch.notmuchmail.org>\r
  69 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  70         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  71 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  72 List-Post: <mailto:notmuch@notmuchmail.org>\r
  73 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  74 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  75         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  76 X-List-Received-Date: Sun, 03 Feb 2013 20:01:34 -0000\r
  77 \r
  78 If I'm reading this right, this patch reduces a 8x memory waste to a\r
  79 4x memory waste, but doesn't actually eliminate the waste.\r
  80 \r
  81 Quoth Robert Mast on Feb 03 at  7:51 pm:\r
  82 > Reading it in detail I thought it allocated way too much memory and\r
  83 > didn't use the full size of the allocated unsigned ints for storing\r
  84 > bits.\r
  85 > \r
  86 > Am I right, and is this the right way to patch code to notmuch?\r
  87 \r
  88 Side-comments about a patch like these should go under the "---" line,\r
  89 while the commit message should explain what a patch does and why.\r
  90 >From your message, I didn't even understand this was fixing a bug; it\r
  91 sounded like a cleanup.\r
  92 \r
  93   http://notmuchmail.org/contributing/#index5h2\r
  94 has a bit to say about commit message contents and the notmuch git\r
  95 history has a lot to say.\r
  96 \r
  97 > \r
  98 > ---\r
  99 >  lib/query.cc |    6 +++---\r
 100 >  1 file changed, 3 insertions(+), 3 deletions(-)\r
 101 > \r
 102 > diff --git a/lib/query.cc b/lib/query.cc\r
 103 > index e9c1a2d..046663a 100644\r
 104 > --- a/lib/query.cc\r
 105 > +++ b/lib/query.cc\r
 106 > @@ -43,8 +43,8 @@ struct _notmuch_doc_id_set {\r
 107 >      unsigned int bound;\r
 108 >  };\r
 109 >  \r
 110 > -#define DOCIDSET_WORD(bit) ((bit) / sizeof (unsigned int))\r
 111 > -#define DOCIDSET_BIT(bit) ((bit) % sizeof (unsigned int))\r
 112 > +#define DOCIDSET_WORD(bit) ((bit) / (sizeof (unsigned int) * CHAR_BIT))\r
 113 > +#define DOCIDSET_BIT(bit) ((bit) % (sizeof (unsigned int) * CHAR_BIT))\r
 114 \r
 115 This is definitely the right thing to do, though a possibly clearer\r
 116 way to fix this would be to change the type of\r
 117 _notmuch_doc_id_set::bitmap to unsigned char.  Then the macros would\r
 118 become simply\r
 119 \r
 120 #define DOCIDSET_WORD(bit) ((bit) / CHAR_BIT)\r
 121 #define DOCIDSET_BIT(bit) ((bit) % CHAR_BIT)\r
 122 \r
 123 >  \r
 124 >  struct visible _notmuch_threads {\r
 125 >      notmuch_query_t *query;\r
 126 > @@ -363,7 +363,7 @@ _notmuch_doc_id_set_init (void *ctx,\r
 127 >  \r
 128 >      for (unsigned int i = 0; i < arr->len; i++)\r
 129 >       max = MAX(max, g_array_index (arr, unsigned int, i));\r
 130 > -    bitmap = talloc_zero_array (ctx, unsigned int, 1 + max / sizeof (*bitmap));\r
 131 > +    bitmap = talloc_zero_array (ctx, unsigned int, (DOCIDSET_WORD(max) + 1) * sizeof (*bitmap));\r
 132 \r
 133 This, however, isn't quite right.  I like your idea of using the\r
 134 macros to compute the size, but the size argument should just be\r
 135   DOCIDSET_WORD(max) + 1\r
 136 since talloc_zero_array expects the number of array elements, not the\r
 137 number of bytes.\r
 138 \r
 139 >  \r
 140 >      if (bitmap == NULL)\r
 141 >       return FALSE;\r