9a/3e5b22a9c575879a05eb22d0cf66948ea415c0

   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 C5D92431FC0\r
   6         for <notmuch@notmuchmail.org>; Wed, 25 Mar 2015 09:39:49 -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 BgZaEHkKtE2x for <notmuch@notmuchmail.org>;\r
  16         Wed, 25 Mar 2015 09:39: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 49D86431FBC\r
  19         for <notmuch@notmuchmail.org>; Wed, 25 Mar 2015 09:39: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 B3CA7100086;\r
  22         Wed, 25 Mar 2015 18:39:24 +0200 (EET)\r
  23 From: Tomi Ollila <tomi.ollila@iki.fi>\r
  24 To: David Bremner <david@tethera.net>, David Bremner <david@tethera.net>,\r
  25         notmuch@notmuchmail.org\r
  26 Subject: Re: [Patch v5 4/8] lib: add "verbose" versions\r
  27         of      notmuch_database_{open, create}\r
  28 In-Reply-To: <1427203451-1540-5-git-send-email-david@tethera.net>\r
  29 References: <1426352554-4383-10-git-send-email-david@tethera.net>\r
  30         <1427203451-1540-1-git-send-email-david@tethera.net>\r
  31         <1427203451-1540-5-git-send-email-david@tethera.net>\r
  32 User-Agent: Notmuch/0.19+92~g402df12 (http://notmuchmail.org) Emacs/24.3.1\r
  33         (x86_64-unknown-linux-gnu)\r
  34 X-Face: HhBM'cA~<r"^Xv\KRN0P{vn'Y"Kd;zg_y3S[4)KSN~s?O\"QPoL\r
  35         $[Xv_BD:i/F$WiEWax}R(MPS`^UaptOGD`*/=@\1lKoVa9tnrg0TW?"r7aRtgk[F\r
  36         !)g;OY^,BjTbr)Np:%c_o'jj,Z\r
  37 Date: Wed, 25 Mar 2015 18:39:24 +0200\r
  38 Message-ID: <m2bnjhuj43.fsf@guru.guru-group.fi>\r
  39 MIME-Version: 1.0\r
  40 Content-Type: text/plain\r
  41 X-BeenThere: notmuch@notmuchmail.org\r
  42 X-Mailman-Version: 2.1.13\r
  43 Precedence: list\r
  44 List-Id: "Use and development of the notmuch mail system."\r
  45         <notmuch.notmuchmail.org>\r
  46 List-Unsubscribe: <http://notmuchmail.org/mailman/options/notmuch>,\r
  47         <mailto:notmuch-request@notmuchmail.org?subject=unsubscribe>\r
  48 List-Archive: <http://notmuchmail.org/pipermail/notmuch>\r
  49 List-Post: <mailto:notmuch@notmuchmail.org>\r
  50 List-Help: <mailto:notmuch-request@notmuchmail.org?subject=help>\r
  51 List-Subscribe: <http://notmuchmail.org/mailman/listinfo/notmuch>,\r
  52         <mailto:notmuch-request@notmuchmail.org?subject=subscribe>\r
  53 X-List-Received-Date: Wed, 25 Mar 2015 16:39:49 -0000\r
  54 \r
  55 On Tue, Mar 24 2015, David Bremner <david@tethera.net> wrote:\r
  56 \r
  57 > The compatibility wrapper ensures that clients calling\r
  58 > notmuch_database_open will receive consistent output for now.\r
  59 >\r
  60 > The changes to notmuch-{new,search} and test/symbol-test are just to\r
  61 > make the test suite pass.\r
  62 >\r
  63 > The use of IGNORE_RESULT is justified by two things. 1) I don't know\r
  64 > what else to do.  2) asprintf guarantees the output string is NULL if\r
  65 > an error occurs, so at least we are not passing garbage back.\r
  66 > ---\r
  67 >  lib/database.cc     | 94 +++++++++++++++++++++++++++++++++++++----------------\r
  68 >  lib/notmuch.h       | 21 ++++++++++++\r
  69 >  notmuch-new.c       |  8 +++--\r
  70 >  notmuch-search.c    |  8 +++--\r
  71 >  test/symbol-test.cc |  6 +++-\r
  72 >  5 files changed, 104 insertions(+), 33 deletions(-)\r
  73 >\r
  74 > diff --git a/lib/database.cc b/lib/database.cc\r
  75 > index 3974e2e..36849d7 100644\r
  76 > --- a/lib/database.cc\r
  77 > +++ b/lib/database.cc\r
  78 > @@ -608,29 +608,39 @@ parse_references (void *ctx,\r
  79 >  notmuch_status_t\r
  80 >  notmuch_database_create (const char *path, notmuch_database_t **database)\r
  81 >  {\r
  82 > +    return notmuch_database_create_verbose (path, database, NULL);\r
  83 \r
  84 Hmm, is is so that nothing is printed if creating database fails, should\r
  85 this provide &message to _verbose function and if message changed, fputs()\r
  86 it (and then free ())?\r
  87 \r
  88 ... after looking below -- notmuch_database_open () does it.\r
  89 \r
  90 > +}\r
  91 > +\r
  92 > +notmuch_status_t\r
  93 > +notmuch_database_create_verbose (const char *path,\r
  94 > +                              notmuch_database_t **database,\r
  95 > +                              char **status_string)\r
  96 > +{\r
  97 >      notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;\r
  98 >      notmuch_database_t *notmuch = NULL;\r
  99 >      char *notmuch_path = NULL;\r
 100 > +    char *message = NULL;\r
 101 >      struct stat st;\r
 102 >      int err;\r
 103 >  \r
 104 >      if (path == NULL) {\r
 105 > -     fprintf (stderr, "Error: Cannot create a database for a NULL path.\n");\r
 106 > +     message = strdup ("Error: Cannot create a database for a NULL path.\n");\r
 107 >       status = NOTMUCH_STATUS_NULL_POINTER;\r
 108 >       goto DONE;\r
 109 >      }\r
 110 >  \r
 111 >      err = stat (path, &st);\r
 112 >      if (err) {\r
 113 > -     fprintf (stderr, "Error: Cannot create database at %s: %s.\n",\r
 114 > -              path, strerror (errno));\r
 115 > +     IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: %s.\n",\r
 116 > +                             path, strerror (errno)));\r
 117 >       status = NOTMUCH_STATUS_FILE_ERROR;\r
 118 >       goto DONE;\r
 119 >      }\r
 120 >  \r
 121 >      if (! S_ISDIR (st.st_mode)) {\r
 122 > -     fprintf (stderr, "Error: Cannot create database at %s: Not a directory.\n",\r
 123 > -              path);\r
 124 > +     IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: "\r
 125 > +                              "Not a directory.\n",\r
 126 > +                              path));\r
 127 >       status = NOTMUCH_STATUS_FILE_ERROR;\r
 128 >       goto DONE;\r
 129 >      }\r
 130 > @@ -640,15 +650,15 @@ notmuch_database_create (const char *path, notmuch_database_t **database)\r
 131 >      err = mkdir (notmuch_path, 0755);\r
 132 >  \r
 133 >      if (err) {\r
 134 > -     fprintf (stderr, "Error: Cannot create directory %s: %s.\n",\r
 135 > -              notmuch_path, strerror (errno));\r
 136 > +     IGNORE_RESULT (asprintf (&message, "Error: Cannot create directory %s: %s.\n",\r
 137 > +                              notmuch_path, strerror (errno)));\r
 138 >       status = NOTMUCH_STATUS_FILE_ERROR;\r
 139 >       goto DONE;\r
 140 >      }\r
 141 >  \r
 142 > -    status = notmuch_database_open (path,\r
 143 > -                                 NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 144 > -                                 &notmuch);\r
 145 > +    status = notmuch_database_open_verbose (path,\r
 146 > +                                         NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 147 > +                                         &notmuch, &message);\r
 148 >      if (status)\r
 149 >       goto DONE;\r
 150 >  \r
 151 > @@ -667,6 +677,8 @@ notmuch_database_create (const char *path, notmuch_database_t **database)\r
 152 >      if (notmuch_path)\r
 153 >       talloc_free (notmuch_path);\r
 154 >  \r
 155 > +    if (message)\r
 156 > +     *status_string = message;\r
 157 \r
 158 Hmm, what if status_string == NULL -- do we have a test for this (or am I\r
 159 missing something ?)\r
 160 \r
 161 >      if (database)\r
 162 >       *database = notmuch;\r
 163 >      else\r
 164 > @@ -767,37 +779,55 @@ notmuch_database_open (const char *path,\r
 165 >                      notmuch_database_mode_t mode,\r
 166 >                      notmuch_database_t **database)\r
 167 >  {\r
 168 > +    char *status_string = NULL;\r
 169 > +    notmuch_status_t status;\r
 170 > +\r
 171 > +    status = notmuch_database_open_verbose(path, mode, database,\r
 172 > +                                        &status_string);\r
 173 > +\r
 174 > +    if (status_string) fputs(status_string, stderr);\r
 175 \r
 176 and  free (status_string);\r
 177 \r
 178 (and fputs (...) (i.e. space which is in all other hunks in this patch :)\r
 179 \r
 180 > +\r
 181 > +    return status;\r
 182 > +}\r
 183 > +\r
 184 > +notmuch_status_t\r
 185 > +notmuch_database_open_verbose (const char *path,\r
 186 > +                            notmuch_database_mode_t mode,\r
 187 > +                            notmuch_database_t **database,\r
 188 > +                            char **status_string)\r
 189 > +{\r
 190 >      notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;\r
 191 >      void *local = talloc_new (NULL);\r
 192 >      notmuch_database_t *notmuch = NULL;\r
 193 >      char *notmuch_path, *xapian_path, *incompat_features;\r
 194 > +    char *message = NULL;\r
 195 >      struct stat st;\r
 196 >      int err;\r
 197 >      unsigned int i, version;\r
 198 >      static int initialized = 0;\r
 199 >  \r
 200 >      if (path == NULL) {\r
 201 > -     fprintf (stderr, "Error: Cannot open a database for a NULL path.\n");\r
 202 > +     message = strdup ("Error: Cannot open a database for a NULL path.\n");\r
 203 >       status = NOTMUCH_STATUS_NULL_POINTER;\r
 204 >       goto DONE;\r
 205 >      }\r
 206 >  \r
 207 >      if (! (notmuch_path = talloc_asprintf (local, "%s/%s", path, ".notmuch"))) {\r
 208 > -     fprintf (stderr, "Out of memory\n");\r
 209 > +     message = strdup ("Out of memory\n");\r
 210 >       status = NOTMUCH_STATUS_OUT_OF_MEMORY;\r
 211 >       goto DONE;\r
 212 >      }\r
 213 >  \r
 214 >      err = stat (notmuch_path, &st);\r
 215 >      if (err) {\r
 216 > -     fprintf (stderr, "Error opening database at %s: %s\n",\r
 217 > -              notmuch_path, strerror (errno));\r
 218 > +     IGNORE_RESULT (asprintf (&message, "Error opening database at %s: %s\n",\r
 219 > +                              notmuch_path, strerror (errno)));\r
 220 >       status = NOTMUCH_STATUS_FILE_ERROR;\r
 221 >       goto DONE;\r
 222 >      }\r
 223 >  \r
 224 >      if (! (xapian_path = talloc_asprintf (local, "%s/%s", notmuch_path, "xapian"))) {\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 > @@ -837,11 +867,11 @@ notmuch_database_open (const char *path,\r
 231 >        * means a dramatically incompatible change. */\r
 232 >       version = notmuch_database_get_version (notmuch);\r
 233 >       if (version > NOTMUCH_DATABASE_VERSION) {\r
 234 > -         fprintf (stderr,\r
 235 > -                  "Error: Notmuch database at %s\n"\r
 236 > -                  "       has a newer database format version (%u) than supported by this\n"\r
 237 > -                  "       version of notmuch (%u).\n",\r
 238 > -                  notmuch_path, version, NOTMUCH_DATABASE_VERSION);\r
 239 > +         IGNORE_RESULT (asprintf (&message,\r
 240 > +                   "Error: Notmuch database at %s\n"\r
 241 > +                   "       has a newer database format version (%u) than supported by this\n"\r
 242 > +                   "       version of notmuch (%u).\n",\r
 243 > +                                  notmuch_path, version, NOTMUCH_DATABASE_VERSION));\r
 244 >           notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;\r
 245 >           notmuch_database_destroy (notmuch);\r
 246 >           notmuch = NULL;\r
 247 > @@ -856,11 +886,11 @@ notmuch_database_open (const char *path,\r
 248 >           version, mode == NOTMUCH_DATABASE_MODE_READ_WRITE ? 'w' : 'r',\r
 249 >           &incompat_features);\r
 250 >       if (incompat_features) {\r
 251 > -         fprintf (stderr,\r
 252 > -                  "Error: Notmuch database at %s\n"\r
 253 > -                  "       requires features (%s)\n"\r
 254 > -                  "       not supported by this version of notmuch.\n",\r
 255 > -                  notmuch_path, incompat_features);\r
 256 > +         IGNORE_RESULT (asprintf (&message,\r
 257 > +             "Error: Notmuch database at %s\n"\r
 258 > +             "       requires features (%s)\n"\r
 259 > +             "       not supported by this version of notmuch.\n",\r
 260 > +                                  notmuch_path, incompat_features));\r
 261 >           notmuch->mode = NOTMUCH_DATABASE_MODE_READ_ONLY;\r
 262 >           notmuch_database_destroy (notmuch);\r
 263 >           notmuch = NULL;\r
 264 > @@ -906,8 +936,8 @@ notmuch_database_open (const char *path,\r
 265 >           notmuch->query_parser->add_prefix (prefix->name, prefix->prefix);\r
 266 >       }\r
 267 >      } catch (const Xapian::Error &error) {\r
 268 > -     fprintf (stderr, "A Xapian exception occurred opening database: %s\n",\r
 269 > -              error.get_msg().c_str());\r
 270 > +     IGNORE_RESULT (asprintf (&message, "A Xapian exception occurred opening database: %s\n",\r
 271 > +                              error.get_msg().c_str()));\r
 272 >       notmuch_database_destroy (notmuch);\r
 273 >       notmuch = NULL;\r
 274 >       status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;\r
 275 > @@ -916,6 +946,9 @@ notmuch_database_open (const char *path,\r
 276 >    DONE:\r
 277 >      talloc_free (local);\r
 278 >  \r
 279 > +    if (status_string && message)\r
 280 > +     *status_string = message;\r
 281 \r
 282 here it is !!! \o/ !!! :D\r
 283 \r
 284 > +\r
 285 >      if (database)\r
 286 >       *database = notmuch;\r
 287 >      else\r
 288 > @@ -1039,13 +1072,18 @@ notmuch_database_compact (const char *path,\r
 289 >      notmuch_database_t *notmuch = NULL;\r
 290 >      struct stat statbuf;\r
 291 >      notmuch_bool_t keep_backup;\r
 292 > +    char *message = NULL;\r
 293 >  \r
 294 >      local = talloc_new (NULL);\r
 295 >      if (! local)\r
 296 >       return NOTMUCH_STATUS_OUT_OF_MEMORY;\r
 297 >  \r
 298 > -    ret = notmuch_database_open (path, NOTMUCH_DATABASE_MODE_READ_WRITE, &notmuch);\r
 299 > +    ret = notmuch_database_open_verbose (path,\r
 300 > +                                      NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 301 > +                                      &notmuch,\r
 302 > +                                      &message);\r
 303 >      if (ret) {\r
 304 > +     if (status_cb) status_cb (message, closure);\r
 305 >       goto DONE;\r
 306 >      }\r
 307 \r
 308 Is this ever printing the message (if any?)\r
 309 \r
 310 >  \r
 311 > diff --git a/lib/notmuch.h b/lib/notmuch.h\r
 312 > index f066245..c671d82 100644\r
 313 > --- a/lib/notmuch.h\r
 314 > +++ b/lib/notmuch.h\r
 315 > @@ -230,6 +230,16 @@ notmuch_status_t\r
 316 >  notmuch_database_create (const char *path, notmuch_database_t **database);\r
 317 >  \r
 318 >  /**\r
 319 > + * Like notmuch_database_create, except optionally return an error\r
 320 > + * message. This message is allocated by malloc and should be freed by\r
 321 > + * the caller.\r
 322 > + */\r
 323 > +notmuch_status_t\r
 324 > +notmuch_database_create_verbose (const char *path,\r
 325 > +                              notmuch_database_t **database,\r
 326 > +                              char **error_message);\r
 327 > +\r
 328 > +/**\r
 329 >   * Database open mode for notmuch_database_open.\r
 330 >   */\r
 331 >  typedef enum {\r
 332 > @@ -279,6 +289,17 @@ notmuch_status_t\r
 333 >  notmuch_database_open (const char *path,\r
 334 >                      notmuch_database_mode_t mode,\r
 335 >                      notmuch_database_t **database);\r
 336 > +/**\r
 337 > + * Like notmuch_database_open, except optionally return an error\r
 338 > + * message. This message is allocated by malloc and should be freed by\r
 339 > + * the caller.\r
 340 > + */\r
 341 > +\r
 342 > +notmuch_status_t\r
 343 > +notmuch_database_open_verbose (const char *path,\r
 344 > +                            notmuch_database_mode_t mode,\r
 345 > +                            notmuch_database_t **database,\r
 346 > +                            char **error_message);\r
 347 >  \r
 348 >  /**\r
 349 >   * Commit changes and close the given notmuch database.\r
 350 > diff --git a/notmuch-new.c b/notmuch-new.c\r
 351 > index ddf42c1..93b70bf 100644\r
 352 > --- a/notmuch-new.c\r
 353 > +++ b/notmuch-new.c\r
 354 > @@ -985,9 +985,13 @@ notmuch_new_command (notmuch_config_t *config, int argc, char *argv[])\r
 355 >           return EXIT_FAILURE;\r
 356 >       add_files_state.total_files = count;\r
 357 >      } else {\r
 358 > -     if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 359 > -                                &notmuch))\r
 360 > +     char *status_string = NULL;\r
 361 > +     if (notmuch_database_open_verbose (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,\r
 362 > +                                        &notmuch, &status_string)) {\r
 363 > +         if (status_string) fputs (status_string, stderr);\r
 364 \r
 365 and free (status_string);\r
 366 \r
 367 > +\r
 368 >           return EXIT_FAILURE;\r
 369 > +     }\r
 370 >  \r
 371 >       if (notmuch_database_needs_upgrade (notmuch)) {\r
 372 >           time_t now = time (NULL);\r
 373 > diff --git a/notmuch-search.c b/notmuch-search.c\r
 374 > index a591d45..d012af3 100644\r
 375 > --- a/notmuch-search.c\r
 376 > +++ b/notmuch-search.c\r
 377 > @@ -545,6 +545,7 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar\r
 378 >  {\r
 379 >      char *query_str;\r
 380 >      unsigned int i;\r
 381 > +    char *status_string = NULL;\r
 382 >  \r
 383 >      switch (ctx->format_sel) {\r
 384 >      case NOTMUCH_FORMAT_TEXT:\r
 385 > @@ -570,9 +571,12 @@ _notmuch_search_prepare (search_context_t *ctx, notmuch_config_t *config, int ar\r
 386 >  \r
 387 >      notmuch_exit_if_unsupported_format ();\r
 388 >  \r
 389 > -    if (notmuch_database_open (notmuch_config_get_database_path (config),\r
 390 > -                            NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch))\r
 391 > +    if (notmuch_database_open_verbose (\r
 392 > +         notmuch_config_get_database_path (config),\r
 393 > +         NOTMUCH_DATABASE_MODE_READ_ONLY, &ctx->notmuch, &status_string)) {\r
 394 > +     if (status_string) fputs (status_string, stderr);\r
 395 \r
 396 diiitto (or do we skip freeing 'small' allocations just before exit)\r
 397 \r
 398 >       return EXIT_FAILURE;\r
 399 > +    }\r
 400 >  \r
 401 >      query_str = query_string_from_args (ctx->notmuch, argc, argv);\r
 402 >      if (query_str == NULL) {\r
 403 > diff --git a/test/symbol-test.cc b/test/symbol-test.cc\r
 404 > index 3e96c03..9f8eea7 100644\r
 405 > --- a/test/symbol-test.cc\r
 406 > +++ b/test/symbol-test.cc\r
 407 > @@ -5,7 +5,11 @@\r
 408 >  \r
 409 >  int main() {\r
 410 >    notmuch_database_t *notmuch;\r
 411 > -  notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch);\r
 412 > +  char *message = NULL;\r
 413 > +\r
 414 > +  if (notmuch_database_open_verbose  ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, &notmuch, &message))\r
 415 > +      if (message) fputs (message, stderr);\r
 416 \r
 417 free... (for consistency)\r
 418 \r
 419 > +\r
 420 >  \r
 421 >    try {\r
 422 >      (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);\r
 423 > -- \r
 424 > 2.1.4\r