b9/ced320a76e20b179947bb277e05b0e44de3a63

   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 CE12C429E26\r
   6         for <notmuch@notmuchmail.org>; Sat, 10 Dec 2011 10:23:01 -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 XRRe73si9Nkj for <notmuch@notmuchmail.org>;\r
  16         Sat, 10 Dec 2011 10:23:01 -0800 (PST)\r
  17 Received: from dmz-mailsec-scanner-1.mit.edu (DMZ-MAILSEC-SCANNER-1.MIT.EDU\r
  18         [18.9.25.12])\r
  19         by olra.theworths.org (Postfix) with ESMTP id 08111431FB6\r
  20         for <notmuch@notmuchmail.org>; Sat, 10 Dec 2011 10:23:00 -0800 (PST)\r
  21 X-AuditID: 1209190c-b7f806d0000008d6-85-4ee3a384bb0b\r
  22 Received: from mailhub-auth-1.mit.edu ( [18.9.21.35])\r
  23         by dmz-mailsec-scanner-1.mit.edu (Symantec Messaging Gateway) with SMTP\r
  24         id 2C.51.02262.483A3EE4; Sat, 10 Dec 2011 13:23:00 -0500 (EST)\r
  25 Received: from outgoing.mit.edu (OUTGOING-AUTH.MIT.EDU [18.7.22.103])\r
  26         by mailhub-auth-1.mit.edu (8.13.8/8.9.2) with ESMTP id pBAIMwsw023402; \r
  27         Sat, 10 Dec 2011 13:22:59 -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.6/8.12.4) with ESMTP id pBAIMtaV028096\r
  32         (version=TLSv1/SSLv3 cipher=AES256-SHA bits=256 verify=NOT);\r
  33         Sat, 10 Dec 2011 13:22:56 -0500 (EST)\r
  34 Received: from amthrax by awakening.csail.mit.edu with local (Exim 4.77)\r
  35         (envelope-from <amdragon@mit.edu>)\r
  36         id 1RZRbA-0002J9-4m; Sat, 10 Dec 2011 13:24:36 -0500\r
  37 Date: Sat, 10 Dec 2011 13:24:36 -0500\r
  38 From: Austin Clements <amdragon@MIT.EDU>\r
  39 To: David Bremner <david@tethera.net>\r
  40 Subject: Re: [RFC PATCH] cli: factor out config handling code to get/set\r
  41  lists.\r
  42 Message-ID: <20111210182436.GE2760@mit.edu>\r
  43 References: <1323539438-15869-1-git-send-email-david@tethera.net>\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: <1323539438-15869-1-git-send-email-david@tethera.net>\r
  48 User-Agent: Mutt/1.5.21 (2010-09-15)\r
  49 X-Brightmail-Tracker:\r
  50  H4sIAAAAAAAAA+NgFupjleLIzCtJLcpLzFFi42IR4hRV1m1Z/NjPYPVPBYuNy34yW9xo7Wa0\r
  51         uH5zJrMDs8evtrnMHs9W3WL22HLoPXMAcxSXTUpqTmZZapG+XQJXxsn1M5kLThpUbDm1hL2B\r
  52         8YRyFyMnh4SAicSc29fZIWwxiQv31rN1MXJxCAnsY5RoujyRGcLZwCixcsdHFgjnJJPE5vtb\r
  53         2CGcJYwSP968YwHpZxFQlZj28DUTiM0moCGxbf9yRhBbBCh+ddtkNhCbWcBeYtHsSWD7hAUC\r
  54         JXbe3ANWzyugLTHh5mpWEFtIwFGi6dA+Zoi4oMTJmU9YIHq1JG78ewlUzwFkS0ss/8cBEuYU\r
  55         cJI403UHbJWogIrElJPb2CYwCs1C0j0LSfcshO4FjMyrGGVTcqt0cxMzc4pTk3WLkxPz8lKL\r
  56         dA31cjNL9FJTSjcxgoNdkmcH45uDSocYBTgYlXh4Pb0e+QmxJpYVV+YeYpTkYFIS5f246LGf\r
  57         EF9SfkplRmJxRnxRaU5q8SFGCQ5mJRHe051AOd6UxMqq1KJ8mJQ0B4uSOG/1rod+QgLpiSWp\r
  58         2ampBalFMFkZDg4lCd4PIEMFi1LTUyvSMnNKENJMHJwgw3mAhj8DqeEtLkjMLc5Mh8ifYlSU\r
  59         Euc9CZIQAElklObB9cKS0StGcaBXhHlfgFTxABMZXPcroMFMQIO/ZD8AGVySiJCSamAsuMDU\r
  60         E6D988UZOd4fpZozQzJjlJZ+U5933Cp3q8We0Ks/uP43512Yrb7ymeJup+Ko6Oys+b+0BFue\r
  61         nD4ZaKE5dfa3Bk6zs/rJWvX9uh8zs/l9TmSal/zJVNsRK5TNFJJbfqufJ5Ah5SS79NEV0cue\r
  62         WInI2q/Uun9Z7vnTf9tun96y8uCmyUeVWIozEg21mIuKEwHSAzUjIQMAAA==\r
  63 Cc: notmuch@notmuchmail.org, David Bremner <bremner@debian.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: Sat, 10 Dec 2011 18:23:02 -0000\r
  77 \r
  78 Deduplicating this code seems like a great idea, but I don't think\r
  79 macros are the way to do it; especially not one that expands to an\r
  80 important top-level construct like a function definition.\r
  81 \r
  82 What about something like\r
  83 \r
  84 const char **\r
  85 notmuch_config_get_user_other_email (notmuch_config_t *config,\r
  86                                      size_t *length)\r
  87 {\r
  88     return _config_get_list (config, "user", "other_email", \r
  89                              &config->user_other_email, length);\r
  90 }\r
  91 \r
  92 where config->user_other_email is a, say, struct _notmuch_config_list\r
  93 that combines together the cached string list and length?\r
  94 _config_get_list would look a lot like what you have now, but outlist\r
  95 would be a struct _notmuch_config_list* instead of a const char*** and\r
  96 it would always set *length (in addition to setting it in the cached\r
  97 value if necessary).\r
  98 \r
  99 Set could be similarly simple\r
 100 \r
 101 void\r
 102 notmuch_config_set_user_other_email (notmuch_config_t *config,\r
 103                                      const char *other_email[],\r
 104                                      size_t length)\r
 105 {\r
 106     _config_set_list (config, "user", "other_email", &config->user_other_email,\r
 107                       other_email, length);\r
 108 }\r
 109 \r
 110 Quoth David Bremner on Dec 10 at  1:50 pm:\r
 111 > From: David Bremner <bremner@debian.org>\r
 112 > \r
 113 > The code is already duplicated once, and I want to add a third\r
 114 > configuration item that is also a list.\r
 115 > ---\r
 116 > \r
 117 > Mainly I am curious if people think using macros to declare these\r
 118 > "getters" and "setters" makes the code less maintainable.\r
 119 > \r
 120 >  notmuch-config.c |  112 +++++++++++++++++++----------------------------------\r
 121 >  1 files changed, 40 insertions(+), 72 deletions(-)\r
 122 > \r
 123 > diff --git a/notmuch-config.c b/notmuch-config.c\r
 124 > index 1a7ed58..33778a7 100644\r
 125 > --- a/notmuch-config.c\r
 126 > +++ b/notmuch-config.c\r
 127 > @@ -520,93 +520,61 @@ notmuch_config_set_user_primary_email (notmuch_config_t *config,\r
 128 >      config->user_primary_email = NULL;\r
 129 >  }\r
 130 >  \r
 131 > -const char **\r
 132 > -notmuch_config_get_user_other_email (notmuch_config_t *config,\r
 133 > -                                  size_t *length)\r
 134 > +static void\r
 135 > +_config_get_list (notmuch_config_t *config,\r
 136 > +               const char *section, const char *key,\r
 137 > +               const char ***outlist, size_t *length)\r
 138 >  {\r
 139 > -    char **emails;\r
 140 > -    size_t emails_length;\r
 141 > +    char **inlist;\r
 142 >      unsigned int i;\r
 143 >  \r
 144 > -    if (config->user_other_email == NULL) {\r
 145 > -     emails = g_key_file_get_string_list (config->key_file,\r
 146 > -                                          "user", "other_email",\r
 147 > -                                          &emails_length, NULL);\r
 148 > -     if (emails) {\r
 149 > -         config->user_other_email = talloc_size (config,\r
 150 > -                                                 sizeof (char *) *\r
 151 > -                                                 (emails_length + 1));\r
 152 > -         for (i = 0; i < emails_length; i++)\r
 153 > -             config->user_other_email[i] = talloc_strdup (config->user_other_email,\r
 154 > -                                                          emails[i]);\r
 155 > -         config->user_other_email[i] = NULL;\r
 156 > -\r
 157 > -         g_strfreev (emails);\r
 158 > -\r
 159 > -         config->user_other_email_length = emails_length;\r
 160 > +    if (*outlist == NULL) {\r
 161 > +     inlist = g_key_file_get_string_list (config->key_file,\r
 162 > +                                          section, key,\r
 163 > +                                          length, NULL);\r
 164 > +     if (inlist) {\r
 165 > +         *outlist = talloc_size (config, sizeof (char *) *\r
 166 > +                                 (*length + 1));\r
 167 > +         for (i = 0; i < *length; i++)\r
 168 > +             (*outlist)[i] = talloc_strdup (*outlist, inlist[i]);\r
 169 > +         (*outlist)[i] = NULL;\r
 170 > +\r
 171 > +         g_strfreev (inlist);\r
 172 > +\r
 173 >       }\r
 174 >      }\r
 175 >  \r
 176 > -    *length = config->user_other_email_length;\r
 177 > -    return config->user_other_email;\r
 178 >  }\r
 179 >  \r
 180 > -void\r
 181 > -notmuch_config_set_user_other_email (notmuch_config_t *config,\r
 182 > -                                  const char *other_email[],\r
 183 > -                                  size_t length)\r
 184 > -{\r
 185 > -    g_key_file_set_string_list (config->key_file,\r
 186 > -                             "user", "other_email",\r
 187 > -                             other_email, length);\r
 188 > -\r
 189 > -    talloc_free (config->user_other_email);\r
 190 > -    config->user_other_email = NULL;\r
 191 > +#define _GET_LIST(list, group, name) \\r
 192 > +const char ** \\r
 193 > +notmuch_config_get_##list (notmuch_config_t *config, size_t *length) \\r
 194 > +{ \\r
 195 > +    _config_get_list (config, group, name, &(config->list),  \\r
 196 > +                      &(config->list##_length));             \\r
 197 > +    *length = config->list##_length;                         \\r
 198 > +    return config->list; \\r
 199 >  }\r
 200 >  \r
 201 > -const char **\r
 202 > -notmuch_config_get_new_tags (notmuch_config_t *config,\r
 203 > -                          size_t *length)\r
 204 > -{\r
 205 > -    char **tags;\r
 206 > -    size_t tags_length;\r
 207 > -    unsigned int i;\r
 208 > +_GET_LIST (user_other_email, "user", "other_email");\r
 209 > +_GET_LIST (new_tags, "new", "tags");\r
 210 >  \r
 211 > -    if (config->new_tags == NULL) {\r
 212 > -     tags = g_key_file_get_string_list (config->key_file,\r
 213 > -                                        "new", "tags",\r
 214 > -                                        &tags_length, NULL);\r
 215 > -     if (tags) {\r
 216 > -         config->new_tags = talloc_size (config,\r
 217 > -                                         sizeof (char *) *\r
 218 > -                                         (tags_length + 1));\r
 219 > -         for (i = 0; i < tags_length; i++)\r
 220 > -             config->new_tags[i] = talloc_strdup (config->new_tags,\r
 221 > -                                                  tags[i]);\r
 222 > -         config->new_tags[i] = NULL;\r
 223 > -\r
 224 > -         g_strfreev (tags);\r
 225 > -\r
 226 > -         config->new_tags_length = tags_length;\r
 227 > -     }\r
 228 > -    }\r
 229 > +#undef _GET_LIST\r
 230 >  \r
 231 > -    *length = config->new_tags_length;\r
 232 > -    return config->new_tags;\r
 233 > +#define _SET_LIST(list, group, name) \\r
 234 > +void \\r
 235 > +notmuch_config_set_##list (notmuch_config_t *config, const char *list[], \\r
 236 > +                        size_t length) \\r
 237 > +{ \\r
 238 > +    g_key_file_set_string_list (config->key_file, group, name, list, length); \\r
 239 > +    talloc_free (config->list);                                               \\r
 240 > +    config->list = NULL;                                                      \\r
 241 >  }\r
 242 >  \r
 243 > -void\r
 244 > -notmuch_config_set_new_tags (notmuch_config_t *config,\r
 245 > -                          const char *new_tags[],\r
 246 > -                          size_t length)\r
 247 > -{\r
 248 > -    g_key_file_set_string_list (config->key_file,\r
 249 > -                             "new", "tags",\r
 250 > -                             new_tags, length);\r
 251 > +_SET_LIST(user_other_email, "user", "other_email");\r
 252 > +_SET_LIST(new_tags, "new", "tags");\r
 253 >  \r
 254 > -    talloc_free (config->new_tags);\r
 255 > -    config->new_tags = NULL;\r
 256 > -}\r
 257 > +#undef _SET_LIST\r
 258 >  \r
 259 >  /* Given a configuration item of the form <group>.<key> return the\r
 260 >   * component group and key. If any error occurs, print a message on\r