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
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
40 Content-Type: text/plain
\r
41 X-BeenThere: notmuch@notmuchmail.org
\r
42 X-Mailman-Version: 2.1.13
\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
55 On Tue, Mar 24 2015, David Bremner <david@tethera.net> wrote:
\r
57 > The compatibility wrapper ensures that clients calling
\r
58 > notmuch_database_open will receive consistent output for now.
\r
60 > The changes to notmuch-{new,search} and test/symbol-test are just to
\r
61 > make the test suite pass.
\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
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
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
80 > notmuch_database_create (const char *path, notmuch_database_t **database)
\r
82 > + return notmuch_database_create_verbose (path, database, NULL);
\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
88 ... after looking below -- notmuch_database_open () does it.
\r
93 > +notmuch_database_create_verbose (const char *path,
\r
94 > + notmuch_database_t **database,
\r
95 > + char **status_string)
\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
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
111 > err = stat (path, &st);
\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
121 > if (! S_ISDIR (st.st_mode)) {
\r
122 > - fprintf (stderr, "Error: Cannot create database at %s: Not a directory.\n",
\r
124 > + IGNORE_RESULT (asprintf (&message, "Error: Cannot create database at %s: "
\r
125 > + "Not a directory.\n",
\r
127 > status = NOTMUCH_STATUS_FILE_ERROR;
\r
130 > @@ -640,15 +650,15 @@ notmuch_database_create (const char *path, notmuch_database_t **database)
\r
131 > err = mkdir (notmuch_path, 0755);
\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
142 > - status = notmuch_database_open (path,
\r
143 > - NOTMUCH_DATABASE_MODE_READ_WRITE,
\r
145 > + status = notmuch_database_open_verbose (path,
\r
146 > + NOTMUCH_DATABASE_MODE_READ_WRITE,
\r
147 > + ¬much, &message);
\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
156 > + *status_string = message;
\r
158 Hmm, what if status_string == NULL -- do we have a test for this (or am I
\r
159 missing something ?)
\r
162 > *database = notmuch;
\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
168 > + char *status_string = NULL;
\r
169 > + notmuch_status_t status;
\r
171 > + status = notmuch_database_open_verbose(path, mode, database,
\r
172 > + &status_string);
\r
174 > + if (status_string) fputs(status_string, stderr);
\r
176 and free (status_string);
\r
178 (and fputs (...) (i.e. space which is in all other hunks in this patch :)
\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
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
197 > unsigned int i, version;
\r
198 > static int initialized = 0;
\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
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
214 > err = stat (notmuch_path, &st);
\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
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
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
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
264 > @@ -906,8 +936,8 @@ notmuch_database_open (const char *path,
\r
265 > notmuch->query_parser->add_prefix (prefix->name, prefix->prefix);
\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
274 > status = NOTMUCH_STATUS_XAPIAN_EXCEPTION;
\r
275 > @@ -916,6 +946,9 @@ notmuch_database_open (const char *path,
\r
277 > talloc_free (local);
\r
279 > + if (status_string && message)
\r
280 > + *status_string = message;
\r
282 here it is !!! \o/ !!! :D
\r
286 > *database = notmuch;
\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
294 > local = talloc_new (NULL);
\r
296 > return NOTMUCH_STATUS_OUT_OF_MEMORY;
\r
298 > - ret = notmuch_database_open (path, NOTMUCH_DATABASE_MODE_READ_WRITE, ¬much);
\r
299 > + ret = notmuch_database_open_verbose (path,
\r
300 > + NOTMUCH_DATABASE_MODE_READ_WRITE,
\r
304 > + if (status_cb) status_cb (message, closure);
\r
308 Is this ever printing the message (if any?)
\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
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
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
329 > * Database open mode for notmuch_database_open.
\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
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
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
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
358 > - if (notmuch_database_open (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,
\r
360 > + char *status_string = NULL;
\r
361 > + if (notmuch_database_open_verbose (db_path, NOTMUCH_DATABASE_MODE_READ_WRITE,
\r
362 > + ¬much, &status_string)) {
\r
363 > + if (status_string) fputs (status_string, stderr);
\r
365 and free (status_string);
\r
368 > return EXIT_FAILURE;
\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
381 > + char *status_string = NULL;
\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
387 > notmuch_exit_if_unsupported_format ();
\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
396 diiitto (or do we skip freeing 'small' allocations just before exit)
\r
398 > return EXIT_FAILURE;
\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
410 > notmuch_database_t *notmuch;
\r
411 > - notmuch_database_open("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much);
\r
412 > + char *message = NULL;
\r
414 > + if (notmuch_database_open_verbose ("fakedb", NOTMUCH_DATABASE_MODE_READ_ONLY, ¬much, &message))
\r
415 > + if (message) fputs (message, stderr);
\r
417 free... (for consistency)
\r
422 > (void) new Xapian::WritableDatabase("./nonexistant", Xapian::DB_OPEN);
\r