c9/ed2968baf2ad645e10dc19d3ac6725964178e1

   1 Return-Path: <tomi.ollila@iki.fi>\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 07DD2431FB6\r
   6         for <notmuch@notmuchmail.org>; Sat, 28 Mar 2015 03:04:50 -0700 (PDT)\r
   7 X-Virus-Scanned: Debian amavisd-new at olra.theworths.org\r
   8 X-Spam-Flag: NO\r
   9 X-Spam-Score: 2.438\r
  10 X-Spam-Level: **\r
  11 X-Spam-Status: No, score=2.438 tagged_above=-999 required=5\r
  12         tests=[DNS_FROM_AHBL_RHSBL=2.438] 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 8r9hAleA44v3 for <notmuch@notmuchmail.org>;\r
  16         Sat, 28 Mar 2015 03:04:46 -0700 (PDT)\r
  17 Received: from guru.guru-group.fi (guru.guru-group.fi [46.183.73.34])\r
  18         by olra.theworths.org (Postfix) with ESMTP id 54D61431FAF\r
  19         for <notmuch@notmuchmail.org>; Sat, 28 Mar 2015 03:04:46 -0700 (PDT)\r
  20 Received: from guru.guru-group.fi (localhost [IPv6:::1])\r
  21         by guru.guru-group.fi (Postfix) with ESMTP id 71B8B100086;\r
  22         Sat, 28 Mar 2015 12:04:22 +0200 (EET)\r
  23 From: Tomi Ollila <tomi.ollila@iki.fi>\r
  24 To: David Bremner <david@tethera.net>, notmuch@notmuchmail.org\r
  25 Subject: Re: [Patch v6 4/8] lib: add "verbose" versions\r
  26         of      notmuch_database_{open, create}\r
  27 In-Reply-To: <1427494320-1483-5-git-send-email-david@tethera.net>\r
  28 References: <1427494320-1483-1-git-send-email-david@tethera.net>\r
  29         <1427494320-1483-5-git-send-email-david@tethera.net>\r
  30 User-Agent: Notmuch/0.19+92~g402df12 (http://notmuchmail.org) Emacs/24.3.1\r
  31         (x86_64-unknown-linux-gnu)\r
  32 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
  33         $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
  34         !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
  35 Date: Sat, 28 Mar 2015 12:04:22 +0200\r
  36 Message-ID: <m2oandigk9.fsf@guru.guru-group.fi>\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.13\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: Sat, 28 Mar 2015 10:04:50 -0000\r
  52 \r
  53 On Sat, Mar 28 2015, David Bremner <david@tethera.net> wrote:\r
  54 \r
  55 > The compatibility wrapper ensures that clients calling\r
  56 > notmuch_database_open will receive consistent output for now.\r
  57 >\r
  58 > The changes to notmuch-{new,search} and test/symbol-test are just to\r
  59 > make the test suite pass.\r
  60 >\r
  61 > The use of IGNORE_RESULT is justified by two things. 1) I don't know\r
  62 > what else to do.  2) asprintf guarantees the output string is NULL if\r
  63 > an error occurs, so at least we are not passing garbage back.\r
  64 > ---\r
  65 >  lib/database.cc     | 109 ++++++++++++++++++++++++++++++++++++++--------------\r
  66 >  lib/notmuch.h       |  21 ++++++++++\r
  67 >  notmuch-new.c       |  11 +++++-\r
  68 >  notmuch-search.c    |  13 ++++++-\r
  69 >  test/symbol-test.cc |   9 ++++-\r
  70 >  5 files changed, 130 insertions(+), 33 deletions(-)\r
  71 >\r
  72 > diff --git a/lib/database.cc b/lib/database.cc\r
  73 > index 3974e2e..5d973b9 100644\r
  74 > --- a/lib/database.cc\r
  75 > +++ b/lib/database.cc\r
  76 > @@ -608,29 +608,50 @@ parse_references (void *ctx,\r
  77 >  notmuch_status_t\r
  78 >  notmuch_database_create (const char *path, notmuch_database_t **database)\r
  79 >  {\r
  80 > +    char *status_string = NULL;\r
  81 > +    notmuch_status_t status;\r
  82 > +\r
  83 > +    status = notmuch_database_create_verbose (path, database,\r
  84 > +                                           &status_string);\r
  85 > +\r
  86 > +    if (status_string) {\r
  87 > +     fputs (status_string, stderr);\r
  88 > +     free (status_string);\r
  89 > +    }\r
  90 > +\r
  91 > +    return status;\r
  92 > +}\r
  93 > +\r
  94 > +notmuch_status_t\r
  95 > +notmuch_database_create_verbose (const char *path,\r
  96 > +                              notmuch_database_t **database,\r
  97 > +                              char **status_string)\r
  98 > +{\r
  99 >      notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;\r
 100 >      notmuch_database_t *notmuch = NULL;\r
 101 >      char *notmuch_path = NULL;\r
 102 > +    char *message = NULL;\r
 103 >      struct stat st;\r
 104 >      int err;\r
 105 >  \r
 106 >      if (path == NULL) {\r
 107 > -     fprintf (stderr, "Error: Cannot create a database for a NULL path.\n");\r
 108 > +     message = strdup ("Error: Cannot create a database for a NULL path.\n");\r
 109 >       status = NOTMUCH_STATUS_NULL_POINTER;\r
 110 >       goto DONE;\r
 111 >      }\r
 112 >  \r
 113 >      err = stat (path, &st);\r
 114 >      if (err) {\r
 115 > -     fprintf (stderr, "Error: Cannot create database at %s: %s.\n",\r
 116 > -              path, strerror (errno));\r
 117 > +     IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: %s.\n",\r
 118 > +                             path, strerror (errno)));\r
 119 >       status = NOTMUCH_STATUS_FILE_ERROR;\r
 120 >       goto DONE;\r
 121 >      }\r
 122 >  \r
 123 >      if (! S_ISDIR (st.st_mode)) {\r
 124 > -     fprintf (stderr, "Error: Cannot create database at %s: Not a directory.\n",\r
 125 > -              path);\r
 126 > +     IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: "\r
 127 > +                              "Not a directory.\n",\r
 128 > +                              path));\r
 129 >       status = NOTMUCH_STATUS_FILE_ERROR;\r
 130 >       goto DONE;\r
 131 >      }\r
 132 > @@ -640,15 +661,15 @@ notmuch_database_create (const char *path, notmuch_database_t **database)\r
 133 >      err = mkdir (notmuch_path, 0755);\r
 134 >  \r
 135 >      if (err) {\r
 136 > -     fprintf (stderr, "Error: Cannot create directory %s: %s.\n",\r
 137 > -              notmuch_path, strerror (errno));\r
 138 > +     IGNORE_RESULT (asprintf (&message, "Error: Cannot create directory %s: %s.\n",\r
 139 > +                              notmuch_path, strerror (errno)));\r
 140 >       status = NOTMUCH_STATUS_FILE_ERROR;\r
 141 >       goto DONE;\r
 142 >      }\r
 143 >  \r
 144 > -    status = notmuch_database_open (path,\r
 145 > -                                 NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 146 > -                                 &notmuch);\r
 147 > +    status = notmuch_database_open_verbose (path,\r
 148 > +                                         NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 149 > +                                         &notmuch, &message);\r
 150 >      if (status)\r
 151 >       goto DONE;\r
 152 >  \r
 153 > @@ -667,6 +688,9 @@ notmuch_database_create (const char *path, notmuch_database_t **database)\r
 154 >      if (notmuch_path)\r
 155 >       talloc_free (notmuch_path);\r
 156 >  \r
 157 > +    if (status_string && message)\r
 158 > +     *status_string = message;\r
 159 \r
 160 This series looks good and tests pass (I had confusion there before seeing\r
 161 one of the last patch moving one output from stderr to stdout of where\r
 162 the test status_cb prints it output)\r
 163 \r
 164 Just that this small piece of code above git pass my sparse sieve, perhaps\r
 165 this could be amended to:\r
 166 \r
 167     if (message) {\r
 168         if (status_string)\r
 169             *status_string = message;\r
 170         else \r
 171             free(message);\r
 172     }\r
 173 \r
 174 \r
 175 IMO either amend or leave to followup patch; just for that I don't\r
 176 want to go through full patch series ;D\r
 177 \r
 178 \r
 179 > +\r
 180 >      if (database)\r
 181 >       *database = notmuch;\r
 182 >      else\r
 183 > @@ -767,37 +791,58 @@ notmuch_database_open (const char *path,\r
 184 >                      notmuch_database_mode_t mode,\r
 185 >                      notmuch_database_t **database)\r
 186 >  {\r
 187 > +    char *status_string = NULL;\r
 188 > +    notmuch_status_t status;\r
 189 > +\r
 190 > +    status = notmuch_database_open_verbose (path, mode, database,\r
 191 > +                                        &status_string);\r
 192 > +\r
 193 > +    if (status_string) {\r
 194 > +     fputs (status_string, stderr);\r
 195 > +     free (status_string);\r
 196 > +    }\r
 197 > +\r
 198 > +    return status;\r
 199 > +}\r
 200 > +\r
 201 > +notmuch_status_t\r
 202 > +notmuch_database_open_verbose (const char *path,\r
 203 > +                            notmuch_database_mode_t mode,\r
 204 > +                            notmuch_database_t **database,\r
 205 > +                            char **status_string)\r
 206 > +{\r
 207 >      notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;\r
 208 >      void *local = talloc_new (NULL);\r
 209 >      notmuch_database_t *notmuch = NULL;\r
 210 >      char *notmuch_path, *xapian_path, *incompat_features;\r
 211 > +    char *message = NULL;\r
 212 >      struct stat st;\r
 213 >      int err;\r
 214 >      unsigned int i, version;\r
 215 >      static int initialized = 0;\r
 216 >  \r
 217 >      if (path == NULL) {\r
 218 > -     fprintf (stderr, "Error: Cannot open a database for a NULL path.\n");\r
 219 > +     message = strdup ("Error: Cannot open a database for a NULL path.\n");\r
 220 >       status = NOTMUCH_STATUS_NULL_POINTER;\r
 221 >       goto DONE;\r
 222 >      }\r
 223 >  \r
 224 >      if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) {\r
 225 > -     fprintf (stderr, "Out of memory\n");\r
 226 > +     message = strdup ("Out of memory\n");\r
 227 >       status = NOTMUCH_STATUS_OUT_OF_MEMORY;\r
 228 >       goto DONE;\r
 229 >      }\r
 230 >  \r
 231 >      err = stat (notmuch_path, &st);\r
 232 >      if (err) {\r
 233 > -     fprintf (stderr, "Error opening database at %s: %s\n",\r
 234 > -              notmuch_path, strerror (errno));\r
 235 > +     IGNORE_RESULT (asprintf (&message, "Error opening database at %s: %s\n",\r
 236 > +                              notmuch_path, strerror (errno)));\r
 237 >       status = NOTMUCH_STATUS_FILE_ERROR;\r
 238 >       goto DONE;\r
 239 >      }\r
 240 >  \r
 241 >      if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {\r
 242 > -     fprintf (stderr, "Out of memory\n");\r
 243 > +     message = strdup ("Out of memory\n");\r
 244 >       status = NOTMUCH_STATUS_OUT_OF_MEMORY;\r
 245 >       goto DONE;\r
 246 >      }\r
 247 > @@ -837,11 +882,11 @@ notmuch_database_open (const char *path,\r
 248 >        * means a dramatically incompatible change. */\r
 249 >       version = notmuch_database_get_version (notmuch);\r
 250 >       if (version > NOTMUCH_DATABASE_VERSION) {\r
 251 > -         fprintf (stderr,\r
 252 > -                  "Error: Notmuch database at %s\n"\r
 253 > -                  "       has a newer database format version (%u) than supported by this\n"\r
 254 > -                  "       version of notmuch (%u).\n",\r
 255 > -                  notmuch_path, version, NOTMUCH_DATABASE_VERSION);\r
 256 > +         IGNORE_RESULT (asprintf (&message,\r
 257 > +                   "Error: Notmuch database at %s\n"\r
 258 > +                   "       has a newer database format version (%u) than supported by this\n"\r
 259 > +                   "       version of notmuch (%u).\n",\r
 260 > +                                  notmuch_path, version, NOTMUCH_DATABASE_VERSION));\r
 261 >           notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;\r
 262 >           notmuch_database_destroy (notmuch);\r
 263 >           notmuch = NULL;\r
 264 > @@ -856,11 +901,11 @@ notmuch_database_open (const char *path,\r
 265 >           version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r',\r
 266 >           &incompat_features);\r
 267 >       if (incompat_features) {\r
 268 > -         fprintf (stderr,\r
 269 > -                  "Error: Notmuch database at %s\n"\r
 270 > -                  "       requires features (%s)\n"\r
 271 > -                  "       not supported by this version of notmuch.\n",\r
 272 > -                  notmuch_path, incompat_features);\r
 273 > +         IGNORE_RESULT (asprintf (&message,\r
 274 > +             "Error: Notmuch database at %s\n"\r
 275 > +             "       requires features (%s)\n"\r
 276 > +             "       not supported by this version of notmuch.\n",\r
 277 > +                                  notmuch_path, incompat_features));\r
 278 >           notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;\r
 279 >           notmuch_database_destroy (notmuch);\r
 280 >           notmuch = NULL;\r
 281 > @@ -906,8 +951,8 @@ notmuch_database_open (const char *path,\r
 282 >           notmuch->query_parser->add_prefix (prefix->name, prefix->prefix);\r
 283 >       }\r
 284 >      } catch (const Xapian::Error &error) {\r
 285 > -     fprintf (stderr, "A Xapian exception occurred opening database: %s\n",\r
 286 > -              error.get_msg().c_str());\r
 287 > +     IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n",\r
 288 > +                              error.get_msg().c_str()));\r
 289 >       notmuch_database_destroy (notmuch);\r
 290 >       notmuch = NULL;\r
 291 >       status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;\r
 292 > @@ -916,6 +961,9 @@ notmuch_database_open (const char *path,\r
 293 >    DONE:\r
 294 >      talloc_free (local);\r
 295 >  \r
 296 > +    if (status_string && message)\r
 297 > +     *status_string = message;\r
 298 \r
 299 Ditto.\r
 300 \r
 301 > +\r
 302 >      if (database)\r
 303 >       *database = notmuch;\r
 304 >      else\r
 305 > @@ -1039,13 +1087,18 @@ notmuch_database_compact (const char *path,\r
 306 >      notmuch_database_t *notmuch = NULL;\r
 307 >      struct stat statbuf;\r
 308 >      notmuch_bool_t keep_backup;\r
 309 > +    char *message = NULL;\r
 310 >  \r
 311 >      local = talloc_new (NULL);\r
 312 >      if (! local)\r
 313 >       return NOTMUCH_STATUS_OUT_OF_MEMORY;\r
 314 >  \r
 315 > -    ret = notmuch_database_open (path, NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch);\r
 316 > +    ret = notmuch_database_open_verbose (path,\r
 317 > +                                      NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 318 > +                                      &notmuch,\r
 319 > +                                      &message);\r
 320 >      if (ret) {\r
 321 > +     if (status_cb) status_cb (message, closure);\r
 322 >       goto DONE;\r
 323 >      }\r
 324 >  \r
 325 > diff --git a/lib/notmuch.h b/lib/notmuch.h\r
 326 > index f066245..c671d82 100644\r
 327 > --- a/lib/notmuch.h\r
 328 > +++ b/lib/notmuch.h\r
 329 > @@ -230,6 +230,16 @@ notmuch_status_t\r
 330 >  notmuch_database_create (const char *path, notmuch_database_t **database);\r
 331 >  \r
 332 >  /**\r
 333 > + * Like notmuch_database_create, except optionally return an error\r
 334 > + * message. This message is allocated by malloc and should be freed by\r
 335 > + * the caller.\r
 336 > + */\r
 337 > +notmuch_status_t\r
 338 > +notmuch_database_create_verbose (const char *path,\r
 339 > +                              notmuch_database_t **database,\r
 340 > +                              char **error_message);\r
 341 > +\r
 342 > +/**\r
 343 >   * Database open mode for notmuch_database_open.\r
 344 >   */\r
 345 >  typedef enum {\r
 346 > @@ -279,6 +289,17 @@ notmuch_status_t\r
 347 >  notmuch_database_open (const char *path,\r
 348 >                      notmuch_database_mode_t mode,\r
 349 >                      notmuch_database_t **database);\r
 350 > +/**\r
 351 > + * Like notmuch_database_open, except optionally return an error\r
 352 > + * message. This message is allocated by malloc and should be freed by\r
 353 > + * the caller.\r
 354 > + */\r
 355 > +\r
 356 > +notmuch_status_t\r
 357 > +notmuch_database_open_verbose (const char *path,\r
 358 > +                            notmuch_database_mode_t mode,\r
 359 > +                            notmuch_database_t **database,\r
 360 > +                            char **error_message);\r
 361 >  \r
 362 >  /**\r
 363 >   * Commit changes and close the given notmuch database.\r
 364 > diff --git a/notmuch-new.c b/notmuch-new.c\r
 365 > index ddf42c1..e6c283e 100644\r
 366 > --- a/notmuch-new.c\r
 367 > +++ b/notmuch-new.c\r
 368 > @@ -985,9 +985,16 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])\r
 369 >           return EXIT_FAILURE;\r
 370 >       add_files_state.total_files = count;\r
 371 >      } else {\r
 372 > -     if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 373 > -                                &notmuch))\r
 374 > +     char *status_string = NULL;\r
 375 > +     if (notmuch_database_open_verbose (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 376 > +                                        &notmuch, &status_string)) {\r
 377 > +         if (status_string) {\r
 378 > +             fputs (status_string, stderr);\r
 379 > +             free (status_string);\r
 380 > +         }\r
 381 > +\r
 382 >           return EXIT_FAILURE;\r
 383 > +     }\r
 384 >  \r
 385 >       if (notmuch_database_needs_upgrade (notmuch)) {\r
 386 >           time_t now = time (NULL);\r
 387 > diff --git a/notmuch-search.c b/notmuch-search.c\r
 388 > index a591d45..b81ac01 100644\r
 389 > --- a/notmuch-search.c\r
 390 > +++ b/notmuch-search.c\r
 391 > @@ -545,6 +545,7 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar\r
 392 >  {\r
 393 >      char *query_str;\r
 394 >      unsigned int i;\r
 395 > +    char *status_string = NULL;\r
 396 >  \r
 397 >      switch (ctx->format_sel) {\r
 398 >      case NOTMUCH_FORMAT_TEXT:\r
 399 > @@ -570,9 +571,17 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar\r
 400 >  \r
 401 >      notmuch_exit_if_unsupported_format ();\r
 402 >  \r
 403 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 404 > -                            NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch))\r
 405 > +    if (notmuch_database_open_verbose (\r
 406 > +         notmuch_config_get_database_path (config),\r
 407 > +         NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch, &status_string)) {\r
 408 > +\r
 409 > +     if (status_string) {\r
 410 > +         fputs (status_string, stderr);\r
 411 > +         free (status_string);\r
 412 > +     }\r
 413 > +\r
 414 >       return EXIT_FAILURE;\r
 415 > +    }\r
 416 >  \r
 417 >      query_str = query_string_from_args (ctx->notmuch, argc, argv);\r
 418 >      if (query_str == NULL) {\r
 419 > diff --git a/test/symbol-test.cc b/test/symbol-test.cc\r
 420 > index 3e96c03..d979f83 100644\r
 421 > --- a/test/symbol-test.cc\r
 422 > +++ b/test/symbol-test.cc\r
 423 > @@ -1,11 +1,18 @@\r
 424 >  #include <stdio.h>\r
 425 > +#include <stdlib.h>\r
 426 >  #include <xapian.h>\r
 427 >  #include <notmuch.h>\r
 428 >  \r
 429 >  \r
 430 >  int main() {\r
 431 >    notmuch_database_t *notmuch;\r
 432 > -  notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch);\r
 433 > +  char *message = NULL;\r
 434 > +\r
 435 > +  if (notmuch_database_open_verbose  ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch, &message))\r
 436 > +      if (message) {\r
 437 > +       fputs (message, stderr);\r
 438 > +       free (message);\r
 439 > +      }\r
 440 >  \r
 441 >    try {\r
 442 >      (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);\r
 443 > -- \r
 444 > 2.1.4\r
 445 >\r
 446 > _______________________________________________\r
 447 > notmuch mailing list\r
 448 > notmuch@notmuchmail.org\r
 449 > http://notmuchmail.org/mailman/listinfo/notmuch\r