79/13b5f86b93edc1d6ecd6cbfda556b8236bd71f

   1 Return-Path: <jani@nikula.org>\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 4FCBE6DE02D6\r
   6  for <notmuch@notmuchmail.org>; Mon,  5 Oct 2015 11:23:01 -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.162\r
  10 X-Spam-Level: \r
  11 X-Spam-Status: No, score=-0.162 tagged_above=-999 required=5\r
  12  tests=[AWL=-0.186, RCVD_IN_DNSWL_LOW=-0.7, RCVD_IN_MSPIKE_H3=-0.01,\r
  13  RCVD_IN_MSPIKE_WL=-0.01, URIBL_SBL=0.644, URIBL_SBL_A=0.1]\r
  14  autolearn=disabled\r
  15 Received: from arlo.cworth.org ([127.0.0.1])\r
  16  by localhost (arlo.cworth.org [127.0.0.1]) (amavisd-new, port 10024)\r
  17  with ESMTP id sF_TxiaM5s5x for <notmuch@notmuchmail.org>;\r
  18  Mon,  5 Oct 2015 11:22:58 -0700 (PDT)\r
  19 Received: from mail-wi0-f180.google.com (mail-wi0-f180.google.com\r
  20  [209.85.212.180])\r
  21  by arlo.cworth.org (Postfix) with ESMTPS id 5C2066DE00CB\r
  22  for <notmuch@notmuchmail.org>; Mon,  5 Oct 2015 11:22:58 -0700 (PDT)\r
  23 Received: by wicfx3 with SMTP id fx3so126769327wic.0\r
  24  for <notmuch@notmuchmail.org>; Mon, 05 Oct 2015 11:22:56 -0700 (PDT)\r
  25 X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed;\r
  26  d=1e100.net; s=20130820;\r
  27  h=x-gm-message-state:from:to:subject:in-reply-to:references\r
  28  :user-agent:date:message-id:mime-version:content-type;\r
  29  bh=MaBSh5nkQWPsHTtGuigAQDXtO9MBh301FZzA3HaQUUU=;\r
  30  b=XfauJtNj6RsQLvVsq8075wxA5Q4i77QvoXIpIjnwd0q7ml587QawgBNN2/031lSl47\r
  31  IEcDbhWzUKI8EJ5eWhie5a6hYHyUu92vMi1iRYMcPYqRZHb1cU64aaj9uLCP6kGggbJL\r
  32  PdloHA7M5HoJ4WP96g4FVTrbaoeGvdm+9XGErRaVsOzpuOu92GJRHUWu7DBrA618Qrxr\r
  33  cVW32ivUJ+2faPu0DBNn6VgYNjWKXyNjWkLf7S6ZWvxX/wduqzOYewNnZsid5e9o/yh9\r
  34  pN+6YFsulMy6P2bUsfFTyTHF6DOMIh+GkjRdJCvoY5BONRiBptunG8GgXHB9uTjWXGRQ\r
  35  mSmQ==\r
  36 X-Gm-Message-State:\r
  37  ALoCoQktcH2I8FBoIeZzAUb+0TAqJuKHzty8RnldrquteSuABt9t5HR1jgBCBqKQwyQWIm+PXXFG\r
  38 X-Received: by 10.194.22.165 with SMTP id e5mr30364182wjf.50.1444069376002;\r
  39  Mon, 05 Oct 2015 11:22:56 -0700 (PDT)\r
  40 Received: from localhost (mobile-access-bcee9f-54.dhcp.inet.fi.\r
  41  [188.238.159.54])\r
  42  by smtp.gmail.com with ESMTPSA id f17sm28158297wjn.38.2015.10.05.11.22.55\r
  43  (version=TLSv1/SSLv3 cipher=OTHER);\r
  44  Mon, 05 Oct 2015 11:22:55 -0700 (PDT)\r
  45 From: Jani Nikula <jani@nikula.org>\r
  46 To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org\r
  47 Subject: Re: [Patch v4 3/9] lib: add versions of n_q_count_{message,\r
  48  threads} with status return\r
  49 In-Reply-To: <1443367923-11867-4-git-send-email-david@tethera.net>\r
  50 References: <1443367923-11867-1-git-send-email-david@tethera.net>\r
  51  <1443367923-11867-4-git-send-email-david@tethera.net>\r
  52 User-Agent: Notmuch/0.20.2+101~g7b7dadb (http://notmuchmail.org) Emacs/24.4.1\r
  53  (x86_64-pc-linux-gnu)\r
  54 Date: Mon, 05 Oct 2015 21:22:29 +0300\r
  55 Message-ID: <87h9m589ay.fsf@nikula.org>\r
  56 MIME-Version: 1.0\r
  57 Content-Type: text/plain\r
  58 X-BeenThere: notmuch@notmuchmail.org\r
  59 X-Mailman-Version: 2.1.18\r
  60 Precedence: list\r
  61 List-Id: "Use and development of the notmuch mail system."\r
  62  <notmuch.notmuchmail.org>\r
  63 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  64  <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  65 List-Archive: <http://notmuchmail.org/pipermail/notmuch/>\r
  66 List-Post: <mailto:notmuch@notmuchmail.org>\r
  67 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  68 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  69  <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  70 X-List-Received-Date: Mon, 05 Oct 2015 18:23:01 -0000\r
  71 \r
  72 On Sun, 27 Sep 2015, David Bremner <david@tethera.net> wrote:\r
  73 > Although I think it's a pretty bad idea to continue using the old API,\r
  74 > this allows both a more gentle transition for clients of the library,\r
  75 > and allows us to break one monolithic change into a series\r
  76 \r
  77 Mostly looks as good as it gets (who would say C makes error handling\r
  78 like this look good?!), a few nitpicks inline below, up to your\r
  79 discretion.\r
  80 \r
  81 BR,\r
  82 Jani.\r
  83 \r
  84 \r
  85 > ---\r
  86 >  lib/database.cc | 13 ++++++++++++-\r
  87 >  lib/notmuch.h   | 48 ++++++++++++++++++++++++++++++++++++++++++------\r
  88 >  lib/query.cc    | 47 +++++++++++++++++++++++++++++++++--------------\r
  89 >  3 files changed, 87 insertions(+), 21 deletions(-)\r
  90 >\r
  91 > diff --git a/lib/database.cc b/lib/database.cc\r
  92 > index dcfad8c..f39d448 100644\r
  93 > --- a/lib/database.cc\r
  94 > +++ b/lib/database.cc\r
  95 > @@ -1410,8 +1410,15 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
  96 >       (NOTMUCH_FEATURE_FILE_TERMS | NOTMUCH_FEATURE_BOOL_FOLDER |\r
  97 >        NOTMUCH_FEATURE_LAST_MOD)) {\r
  98 >       query = notmuch_query_create (notmuch, "");\r
  99 > -     total += notmuch_query_count_messages (query);\r
 100 > +     unsigned msg_count;\r
 101 > +\r
 102 > +     status = notmuch_query_count_messages_st (query, &msg_count);\r
 103 > +     if (status)\r
 104 > +         goto DONE;\r
 105 > +\r
 106 > +     total += msg_count;\r
 107 >       notmuch_query_destroy (query);\r
 108 > +     query = NULL;\r
 109 >      }\r
 110 >      if (new_features & NOTMUCH_FEATURE_DIRECTORY_DOCS) {\r
 111 >       t_end = db->allterms_end ("XTIMESTAMP");\r
 112 > @@ -1492,6 +1499,7 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 113 >       }\r
 114 >  \r
 115 >       notmuch_query_destroy (query);\r
 116 > +     query = NULL;\r
 117 >      }\r
 118 >  \r
 119 >      /* Perform per-directory upgrades. */\r
 120 > @@ -1612,6 +1620,9 @@ notmuch_database_upgrade (notmuch_database_t *notmuch,\r
 121 >       sigaction (SIGALRM, &action, NULL);\r
 122 >      }\r
 123 >  \r
 124 > +    if (query)\r
 125 > +     notmuch_query_destroy (query);\r
 126 > +\r
 127 >      talloc_free (local);\r
 128 >      return status;\r
 129 >  }\r
 130 > diff --git a/lib/notmuch.h b/lib/notmuch.h\r
 131 > index 8775683..2fb778b 100644\r
 132 > --- a/lib/notmuch.h\r
 133 > +++ b/lib/notmuch.h\r
 134 > @@ -984,10 +984,26 @@ notmuch_threads_destroy (notmuch_threads_t *threads);\r
 135 >   * This function performs a search and returns the number of matching\r
 136 >   * messages.\r
 137 >   *\r
 138 > - * If a Xapian exception occurs, this function may return 0 (after\r
 139 > - * printing a message).\r
 140 > + * @returns\r
 141 > + *\r
 142 > + * NOTMUCH_STATUS_SUCCESS: query complete successfully.\r
 143 \r
 144 complete_d_?\r
 145 \r
 146 > + *\r
 147 > + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured. The\r
 148 > + *      value of *count is not defined.\r
 149 >   */\r
 150 > -unsigned\r
 151 > +notmuch_status_t\r
 152 > +notmuch_query_count_messages_st (notmuch_query_t *query, unsigned *count);\r
 153 \r
 154 I'd use "unsigned int", but matter of style.\r
 155 \r
 156 Do we care about documenting at which versions the new functions have\r
 157 been introduced? Doxygen supports @since.\r
 158 \r
 159 > +\r
 160 > +/**\r
 161 > + * like notmuch_query_count_messages_st, but without a status return\r
 162 \r
 163 Period at the end.\r
 164 \r
 165 > + *\r
 166 > + * May return 0 in the case of errors.\r
 167 > + *\r
 168 > + * @deprecated Deprecated since libnotmuch 4.3 (notmuch 0.21). Please\r
 169 > + * use notmuch_query_count_messages_st instead.\r
 170 > + */\r
 171 > +NOTMUCH_DEPRECATED(4,3)\r
 172 > +unsigned int\r
 173 >  notmuch_query_count_messages (notmuch_query_t *query);\r
 174 >  \r
 175 >  /**\r
 176 > @@ -998,11 +1014,31 @@ notmuch_query_count_messages (notmuch_query_t *query);\r
 177 >   * search.\r
 178 >   *\r
 179 >   * Note that this is a significantly heavier operation than\r
 180 > - * notmuch_query_count_messages().\r
 181 > + * notmuch_query_count_messages{_st}().\r
 182 \r
 183 I think doxygen can do some hyperlinking here if you use the proper\r
 184 function name(s). On output formats we don't use, but anyway...\r
 185 \r
 186 > + *\r
 187 > + * @returns\r
 188 >   *\r
 189 > - * If an error occurs, this function may return 0.\r
 190 > + * NOTMUCH_STATUS_OUT_OF_MEMORY: Memory allocation failed. The value\r
 191 > + *      of *count is not defined\r
 192 > +\r
 193 > + * NOTMUCH_STATUS_SUCCESS: query complete successfully.\r
 194 \r
 195 complete_d_?\r
 196 \r
 197 > + *\r
 198 > + * NOTMUCH_STATUS_XAPIAN_EXCEPTION: a Xapian exception occured. The\r
 199 > + *      value of *count is not defined.\r
 200 >   */\r
 201 > -unsigned\r
 202 > +notmuch_status_t\r
 203 > +notmuch_query_count_threads_st (notmuch_query_t *query, unsigned *count);\r
 204 > +\r
 205 > +/**\r
 206 > + * like notmuch_query_count_threads, but without a status return.\r
 207 > + *\r
 208 > + * May return 0 in case of errors.\r
 209 > + *\r
 210 > + * @deprecated Deprecated as of libnotmuch 4.3 (notmuch 0.21). Please\r
 211 > + * use notmuch_query_count_threads_st instead.\r
 212 > + */\r
 213 > +NOTMUCH_DEPRECATED(4,3)\r
 214 > +unsigned int\r
 215 >  notmuch_query_count_threads (notmuch_query_t *query);\r
 216 >  \r
 217 >  /**\r
 218 > diff --git a/lib/query.cc b/lib/query.cc\r
 219 > index 8cf0a07..e627bfc 100644\r
 220 > --- a/lib/query.cc\r
 221 > +++ b/lib/query.cc\r
 222 > @@ -541,9 +541,19 @@ notmuch_threads_destroy (notmuch_threads_t *threads)\r
 223 >      talloc_free (threads);\r
 224 >  }\r
 225 >  \r
 226 > -unsigned\r
 227 > +unsigned int\r
 228 >  notmuch_query_count_messages (notmuch_query_t *query)\r
 229 >  {\r
 230 > +    notmuch_status_t status;\r
 231 > +    unsigned int count;\r
 232 > +\r
 233 > +    status = notmuch_query_count_messages_st (query, &count);\r
 234 > +    return status ? 0 : count;\r
 235 > +}\r
 236 > +\r
 237 > +notmuch_status_t\r
 238 > +notmuch_query_count_messages_st (notmuch_query_t *query, unsigned *count_out)\r
 239 > +{\r
 240 >      notmuch_database_t *notmuch = query->notmuch;\r
 241 >      const char *query_string = query->query_string;\r
 242 >      Xapian::doccount count = 0;\r
 243 > @@ -605,35 +615,44 @@ notmuch_query_count_messages (notmuch_query_t *query)\r
 244 >                              "Query string was: %s\n",\r
 245 >                              error.get_msg().c_str(),\r
 246 >                              query->query_string);\r
 247 > -\r
 248 > +     return NOTMUCH_STATUS_XAPIAN_EXCEPTION;\r
 249 >      }\r
 250 >  \r
 251 > -    return count;\r
 252 > +    *count_out = count;\r
 253 > +    return NOTMUCH_STATUS_SUCCESS;\r
 254 >  }\r
 255 >  \r
 256 >  unsigned\r
 257 >  notmuch_query_count_threads (notmuch_query_t *query)\r
 258 >  {\r
 259 > +    notmuch_status_t status;\r
 260 > +    unsigned int count;\r
 261 > +\r
 262 > +    status = notmuch_query_count_threads_st (query, &count);\r
 263 > +    return status ? 0 : count;\r
 264 > +}\r
 265 > +\r
 266 > +notmuch_status_t\r
 267 > +notmuch_query_count_threads_st (notmuch_query_t *query, unsigned *count)\r
 268 > +{\r
 269 >      notmuch_messages_t *messages;\r
 270 >      GHashTable *hash;\r
 271 > -    unsigned int count;\r
 272 >      notmuch_sort_t sort;\r
 273 > -    notmuch_status_t status;\r
 274 > +    notmuch_status_t ret = NOTMUCH_STATUS_SUCCESS;\r
 275 >  \r
 276 >      sort = query->sort;\r
 277 >      query->sort = NOTMUCH_SORT_UNSORTED;\r
 278 > -    status = notmuch_query_search_messages_st (query, &messages);\r
 279 > -    if (status)\r
 280 > -     return 0;\r
 281 > -\r
 282 > +    ret = notmuch_query_search_messages_st (query, &messages);\r
 283 > +    if (ret)\r
 284 > +     return ret;\r
 285 >      query->sort = sort;\r
 286 >      if (messages == NULL)\r
 287 > -     return 0;\r
 288 > +     return NOTMUCH_STATUS_XAPIAN_EXCEPTION;\r
 289 >  \r
 290 >      hash = g_hash_table_new_full (g_str_hash, g_str_equal, NULL, NULL);\r
 291 >      if (hash == NULL) {\r
 292 >       talloc_free (messages);\r
 293 > -     return 0;\r
 294 > +     return NOTMUCH_STATUS_OUT_OF_MEMORY;\r
 295 >      }\r
 296 >  \r
 297 >      while (notmuch_messages_valid (messages)) {\r
 298 > @@ -642,7 +661,7 @@ notmuch_query_count_threads (notmuch_query_t *query)\r
 299 >       char *thread_id_copy = talloc_strdup (messages, thread_id);\r
 300 >       if (unlikely (thread_id_copy == NULL)) {\r
 301 >           notmuch_message_destroy (message);\r
 302 > -         count = 0;\r
 303 > +         ret = NOTMUCH_STATUS_OUT_OF_MEMORY;\r
 304 >           goto DONE;\r
 305 >       }\r
 306 >       g_hash_table_insert (hash, thread_id_copy, NULL);\r
 307 > @@ -650,13 +669,13 @@ notmuch_query_count_threads (notmuch_query_t *query)\r
 308 >       notmuch_messages_move_to_next (messages);\r
 309 >      }\r
 310 >  \r
 311 > -    count = g_hash_table_size (hash);\r
 312 > +    *count = g_hash_table_size (hash);\r
 313 >  \r
 314 >    DONE:\r
 315 >      g_hash_table_unref (hash);\r
 316 >      talloc_free (messages);\r
 317 >  \r
 318 > -    return count;\r
 319 > +    return ret;\r
 320 >  }\r
 321 >  \r
 322 >  notmuch_database_t *\r
 323 > -- \r
 324 > 2.5.3\r
 325 >\r
 326 > _______________________________________________\r
 327 > notmuch mailing list\r
 328 > notmuch@notmuchmail.org\r
 329 > http://notmuchmail.org/mailman/listinfo/notmuch\r