c1/954d986c2cf06f80e6b4ed8c54724c1ee9fe46

   1 Return-Path: <david@tethera.net>\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 arlo.cworth.org (Postfix) with ESMTP id 6173B6DE00CB\r
   6  for <notmuch@notmuchmail.org>; Sat, 29 Aug 2015 18:51:02 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at cworth.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 0.118\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=0.118 tagged_above=-999 required=5 tests=[AWL=0.118]\r
  12  autolearn=disabled\r
  13 Received: from arlo.cworth.org ([127.0.0.1])\r
  14  by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024)\r
  15  with ESMTP id omYwb3QU_2zq for <notmuch@notmuchmail.org>;\r
  16  Sat, 29 Aug 2015 18:51:00 -0700 (PDT)\r
  17 Received: from gitolite.debian.net (gitolite.debian.net [87.98.215.224])\r
  18  by arlo.cworth.org (Postfix) with ESMTPS id 9A3726DE005F\r
  19  for <notmuch@notmuchmail.org>; Sat, 29 Aug 2015 18:51:00 -0700 (PDT)\r
  20 Received: from remotemail by gitolite.debian.net with local (Exim 4.80)\r
  21  (envelope-from <david@tethera.net>)\r
  22  id 1ZVrk9-0005Vn-Fe; Sun, 30 Aug 2015 01:49:13 +0000\r
  23 Received: (nullmailer pid 19707 invoked by uid 1000); Sun, 30 Aug 2015\r
  24  01:48:53 -0000\r
  25 From: David Bremner <david@tethera.net>\r
  26 To: Jani Nikula <jani@nikula.org>, notmuch@notmuchmail.org\r
  27 Subject: Re: [RFC PATCH 4/5] cli: change the data structure for notmuch\r
  28  address deduplication\r
  29 In-Reply-To:\r
  30  <ffe5975fd070d3e8e22602b01ceb8bf0d7d47ae0.1440859765.git.jani@nikula.org>\r
  31 References: <cover.1440859765.git.jani@nikula.org>\r
  32  <ffe5975fd070d3e8e22602b01ceb8bf0d7d47ae0.1440859765.git.jani@nikula.org>\r
  33 User-Agent: Notmuch/0.20.2+60~gcb08a2e (http://notmuchmail.org) Emacs/24.5.1\r
  34  (x86_64-pc-linux-gnu)\r
  35 Date: Sat, 29 Aug 2015 22:48:53 -0300\r
  36 Message-ID: <87a8t9tuka.fsf@maritornes.cs.unb.ca>\r
  37 MIME-Version: 1.0\r
  38 Content-Type: text/plain\r
  39 X-BeenThere: notmuch@notmuchmail.org\r
  40 X-Mailman-Version: 2.1.18\r
  41 Precedence: list\r
  42 List-Id: "Use and development of the notmuch mail system."\r
  43  <notmuch.notmuchmail.org>\r
  44 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  45  <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  46 List-Archive: <http://notmuchmail.org/pipermail/notmuch/>\r
  47 List-Post: <mailto:notmuch@notmuchmail.org>\r
  48 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  49 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  50  <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  51 X-List-Received-Date: Sun, 30 Aug 2015 01:51:02 -0000\r
  52 \r
  53 Jani Nikula <jani@nikula.org> writes:\r
  54 \r
  55 >  \r
  56 > +static int\r
  57 > +strcase_equal (const void *a, const void *b)\r
  58 > +{\r
  59 > +    return strcasecmp (a, b) == 0;\r
  60 > +}\r
  61 > +\r
  62 > +static unsigned int\r
  63 > +strcase_hash (const void *ptr)\r
  64 > +{\r
  65 > +    const char *s = ptr;\r
  66 > +\r
  67 > +    /* This is the djb2 hash. */\r
  68 > +    unsigned int hash = 5381;\r
  69 > +    while (s && *s) {\r
  70 > +     hash = ((hash << 5) + hash) + tolower (*s);\r
  71 > +     s++;\r
  72 > +    }\r
  73 > +\r
  74 > +    return hash;\r
  75 > +}\r
  76 > +\r
  77 \r
  78 as discussed, these functions probably need to be factored out into\r
  79 libutil.\r
  80 \r
  81 > +     l = g_list_find_custom (list, mailbox, mailbox_compare);\r
  82 > +     if (l) {\r
  83 > +         talloc_free (mailbox);\r
  84 > +         mailbox = l->data;\r
  85 > +         mailbox->count++;\r
  86 > +         return TRUE;\r
  87 > +     }\r
  88 \r
  89 I found this use of mailbox as a temporary variable confusing; despite\r
  90 the obvious return I thought it might have something to do with the\r
  91 g_list_append below.  Maybe just make a block scope temporary variable?\r
  92 \r
  93 > +\r
  94 > +     g_list_append (list, mailbox);\r
  95 > +     return FALSE;\r
  96 >      }\r
  97 >  \r
  98 > -    mailbox = new_mailbox (ctx->format, name, addr);\r
  99 > -    if (! mailbox)\r
 100 > +    key = talloc_strdup (ctx->format, addr);\r
 101 > +    if (! key)\r
 102 >       return FALSE;\r
 103 \r
 104 I guess this doesn't make the error handling worse; both old and new\r
 105 code silently ignore OOM if I understand correctly. Do you happen to\r
 106 understand the original choice of using ctx->format rather than that\r
 107 ctx->notmuch for a talloc parent? it doesn't seem to get deallocated any\r
 108 earlier.\r
 109 \r
 110 d\r
 111 \r
 112 \r