[Patch v6 4/8] lib: add "verbose" versions of notmuch_database_{open,create}

2015-03-29 Thread David Bremner 
Tomi Ollila  writes:

>
> Just that this small piece of code above git pass my sparse sieve, perhaps
> this could be amended to:
>
> if (message) {
> if (status_string)
> *status_string = message;
> else 
> free(message);
> }
>

I amended and pushed. Thanks for all the reviewing.

d

[Patch v6 4/8] lib: add "verbose" versions of notmuch_database_{open, create}

2015-03-28 Thread Tomi Ollila 
On Sat, Mar 28 2015, David Bremner  wrote:

> The compatibility wrapper ensures that clients calling
> notmuch_database_open will receive consistent output for now.
>
> The changes to notmuch-{new,search} and test/symbol-test are just to
> make the test suite pass.
>
> The use of IGNORE_RESULT is justified by two things. 1) I don't know
> what else to do.  2) asprintf guarantees the output string is NULL if
> an error occurs, so at least we are not passing garbage back.
> ---
>  lib/database.cc | 109 
> ++--
>  lib/notmuch.h   |  21 ++
>  notmuch-new.c   |  11 +-
>  notmuch-search.c|  13 ++-
>  test/symbol-test.cc |   9 -
>  5 files changed, 130 insertions(+), 33 deletions(-)
>
> diff --git a/lib/database.cc b/lib/database.cc
> index 3974e2e..5d973b9 100644
> --- a/lib/database.cc
> +++ b/lib/database.cc
> @@ -608,29 +608,50 @@ parse_references (void *ctx,
>  notmuch_status_t
>  notmuch_database_create (const char *path, notmuch_database_t **database)
>  {
> +char *status_string = NULL;
> +notmuch_status_t status;
> +
> +status = notmuch_database_create_verbose (path, database,
> +   _string);
> +
> +if (status_string) {
> + fputs (status_string, stderr);
> + free (status_string);
> +}
> +
> +return status;
> +}
> +
> +notmuch_status_t
> +notmuch_database_create_verbose (const char *path,
> +  notmuch_database_t **database,
> +  char **status_string)
> +{
>  notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
>  notmuch_database_t *notmuch = NULL;
>  char *notmuch_path = NULL;
> +char *message = NULL;
>  struct stat st;
>  int err;
>  
>  if (path == NULL) {
> - fprintf (stderr, "Error: Cannot create a database for a NULL path.\n");
> + message = strdup ("Error: Cannot create a database for a NULL path.\n");
>   status = NOTMUCH_STATUS_NULL_POINTER;
>   goto DONE;
>  }
>  
>  err = stat (path, );
>  if (err) {
> - fprintf (stderr, "Error: Cannot create database at %s: %s.\n",
> -  path, strerror (errno));
> + IGNORE_RESULT (asprintf (, "Error: Cannot create database at 
> %s: %s.\n",
> + path, strerror (errno)));
>   status = NOTMUCH_STATUS_FILE_ERROR;
>   goto DONE;
>  }
>  
>  if (! S_ISDIR (st.st_mode)) {
> - fprintf (stderr, "Error: Cannot create database at %s: Not a 
> directory.\n",
> -  path);
> + IGNORE_RESULT (asprintf (, "Error: Cannot create database at 
> %s: "
> +  "Not a directory.\n",
> +  path));
>   status = NOTMUCH_STATUS_FILE_ERROR;
>   goto DONE;
>  }
> @@ -640,15 +661,15 @@ notmuch_database_create (const char *path, 
> notmuch_database_t **database)
>  err = mkdir (notmuch_path, 0755);
>  
>  if (err) {
> - fprintf (stderr, "Error: Cannot create directory %s: %s.\n",
> -  notmuch_path, strerror (errno));
> + IGNORE_RESULT (asprintf (, "Error: Cannot create directory %s: 
> %s.\n",
> +  notmuch_path, strerror (errno)));
>   status = NOTMUCH_STATUS_FILE_ERROR;
>   goto DONE;
>  }
>  
> -status = notmuch_database_open (path,
> - NOTMUCH_DATABASE_MODE_READ_WRITE,
> - );
> +status = notmuch_database_open_verbose (path,
> + NOTMUCH_DATABASE_MODE_READ_WRITE,
> + , );
>  if (status)
>   goto DONE;
>  
> @@ -667,6 +688,9 @@ notmuch_database_create (const char *path, 
> notmuch_database_t **database)
>  if (notmuch_path)
>   talloc_free (notmuch_path);
>  
> +if (status_string && message)
> + *status_string = message;

This series looks good and tests pass (I had confusion there before seeing
one of the last patch moving one output from stderr to stdout of where
the test status_cb prints it output)

Just that this small piece of code above git pass my sparse sieve, perhaps
this could be amended to:

if (message) {
if (status_string)
*status_string = message;
else 
free(message);
}


IMO either amend or leave to followup patch; just for that I don't
want to go through full patch series ;D


> +
>  if (database)
>   *database = notmuch;
>  else
> @@ -767,37 +791,58 @@ notmuch_database_open (const char *path,
>  notmuch_database_mode_t mode,
>  notmuch_database_t **database)
>  {
> +char *status_string = NULL;
> +notmuch_status_t status;
> +
> +status = notmuch_database_open_verbose (path, mode, database,
> +_string);
> +
> +if (status_string) {
> + fputs (status_string, stderr);
> +

[Patch v6 4/8] lib: add "verbose" versions of notmuch_database_{open,create}

2015-03-28 Thread David Bremner 
The compatibility wrapper ensures that clients calling
notmuch_database_open will receive consistent output for now.

The changes to notmuch-{new,search} and test/symbol-test are just to
make the test suite pass.

The use of IGNORE_RESULT is justified by two things. 1) I don't know
what else to do.  2) asprintf guarantees the output string is NULL if
an error occurs, so at least we are not passing garbage back.
---
 lib/database.cc | 109 ++--
 lib/notmuch.h   |  21 ++
 notmuch-new.c   |  11 +-
 notmuch-search.c|  13 ++-
 test/symbol-test.cc |   9 -
 5 files changed, 130 insertions(+), 33 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 3974e2e..5d973b9 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -608,29 +608,50 @@ parse_references (void *ctx,
 notmuch_status_t
 notmuch_database_create (const char *path, notmuch_database_t **database)
 {
+char *status_string = NULL;
+notmuch_status_t status;
+
+status = notmuch_database_create_verbose (path, database,
+ _string);
+
+if (status_string) {
+   fputs (status_string, stderr);
+   free (status_string);
+}
+
+return status;
+}
+
+notmuch_status_t
+notmuch_database_create_verbose (const char *path,
+notmuch_database_t **database,
+char **status_string)
+{
 notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 notmuch_database_t *notmuch = NULL;
 char *notmuch_path = NULL;
+char *message = NULL;
 struct stat st;
 int err;

 if (path == NULL) {
-   fprintf (stderr, "Error: Cannot create a database for a NULL path.\n");
+   message = strdup ("Error: Cannot create a database for a NULL path.\n");
status = NOTMUCH_STATUS_NULL_POINTER;
goto DONE;
 }

 err = stat (path, );
 if (err) {
-   fprintf (stderr, "Error: Cannot create database at %s: %s.\n",
-path, strerror (errno));
+   IGNORE_RESULT (asprintf (, "Error: Cannot create database at 
%s: %s.\n",
+   path, strerror (errno)));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }

 if (! S_ISDIR (st.st_mode)) {
-   fprintf (stderr, "Error: Cannot create database at %s: Not a 
directory.\n",
-path);
+   IGNORE_RESULT (asprintf (, "Error: Cannot create database at 
%s: "
+"Not a directory.\n",
+path));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
@@ -640,15 +661,15 @@ notmuch_database_create (const char *path, 
notmuch_database_t **database)
 err = mkdir (notmuch_path, 0755);

 if (err) {
-   fprintf (stderr, "Error: Cannot create directory %s: %s.\n",
-notmuch_path, strerror (errno));
+   IGNORE_RESULT (asprintf (, "Error: Cannot create directory %s: 
%s.\n",
+notmuch_path, strerror (errno)));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }

-status = notmuch_database_open (path,
-   NOTMUCH_DATABASE_MODE_READ_WRITE,
-   );
+status = notmuch_database_open_verbose (path,
+   NOTMUCH_DATABASE_MODE_READ_WRITE,
+   , );
 if (status)
goto DONE;

@@ -667,6 +688,9 @@ notmuch_database_create (const char *path, 
notmuch_database_t **database)
 if (notmuch_path)
talloc_free (notmuch_path);

+if (status_string && message)
+   *status_string = message;
+
 if (database)
*database = notmuch;
 else
@@ -767,37 +791,58 @@ notmuch_database_open (const char *path,
   notmuch_database_mode_t mode,
   notmuch_database_t **database)
 {
+char *status_string = NULL;
+notmuch_status_t status;
+
+status = notmuch_database_open_verbose (path, mode, database,
+  _string);
+
+if (status_string) {
+   fputs (status_string, stderr);
+   free (status_string);
+}
+
+return status;
+}
+
+notmuch_status_t
+notmuch_database_open_verbose (const char *path,
+  notmuch_database_mode_t mode,
+  notmuch_database_t **database,
+  char **status_string)
+{
 notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 void *local = talloc_new (NULL);
 notmuch_database_t *notmuch = NULL;
 char *notmuch_path, *xapian_path, *incompat_features;
+char *message = NULL;
 struct stat st;
 int err;
 unsigned int i, version;
 static int initialized = 0;

 if (path == NULL) {
-   fprintf (stderr, "Error: Cannot open a database for a NULL path.\n");
+   message = strdup

Re: [Patch v6 4/8] lib: add verbose versions of notmuch_database_{open,create}

2015-03-28 Thread David Bremner 
Tomi Ollila tomi.oll...@iki.fi writes:


 Just that this small piece of code above git pass my sparse sieve, perhaps
 this could be amended to:

 if (message) {
 if (status_string)
 *status_string = message;
 else 
 free(message);
 }


I amended and pushed. Thanks for all the reviewing.

d
___
notmuch mailing list
notmuch@notmuchmail.org
http://notmuchmail.org/mailman/listinfo/notmuch

Re: [Patch v6 4/8] lib: add verbose versions of notmuch_database_{open, create}

2015-03-28 Thread Tomi Ollila 
On Sat, Mar 28 2015, David Bremner da...@tethera.net wrote:

 The compatibility wrapper ensures that clients calling
 notmuch_database_open will receive consistent output for now.

 The changes to notmuch-{new,search} and test/symbol-test are just to
 make the test suite pass.

 The use of IGNORE_RESULT is justified by two things. 1) I don't know
 what else to do.  2) asprintf guarantees the output string is NULL if
 an error occurs, so at least we are not passing garbage back.
 ---
  lib/database.cc | 109 
 ++--
  lib/notmuch.h   |  21 ++
  notmuch-new.c   |  11 +-
  notmuch-search.c|  13 ++-
  test/symbol-test.cc |   9 -
  5 files changed, 130 insertions(+), 33 deletions(-)

 diff --git a/lib/database.cc b/lib/database.cc
 index 3974e2e..5d973b9 100644
 --- a/lib/database.cc
 +++ b/lib/database.cc
 @@ -608,29 +608,50 @@ parse_references (void *ctx,
  notmuch_status_t
  notmuch_database_create (const char *path, notmuch_database_t **database)
  {
 +char *status_string = NULL;
 +notmuch_status_t status;
 +
 +status = notmuch_database_create_verbose (path, database,
 +   status_string);
 +
 +if (status_string) {
 + fputs (status_string, stderr);
 + free (status_string);
 +}
 +
 +return status;
 +}
 +
 +notmuch_status_t
 +notmuch_database_create_verbose (const char *path,
 +  notmuch_database_t **database,
 +  char **status_string)
 +{
  notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
  notmuch_database_t *notmuch = NULL;
  char *notmuch_path = NULL;
 +char *message = NULL;
  struct stat st;
  int err;
  
  if (path == NULL) {
 - fprintf (stderr, Error: Cannot create a database for a NULL path.\n);
 + message = strdup (Error: Cannot create a database for a NULL path.\n);
   status = NOTMUCH_STATUS_NULL_POINTER;
   goto DONE;
  }
  
  err = stat (path, st);
  if (err) {
 - fprintf (stderr, Error: Cannot create database at %s: %s.\n,
 -  path, strerror (errno));
 + IGNORE_RESULT (asprintf (message, Error: Cannot create database at 
 %s: %s.\n,
 + path, strerror (errno)));
   status = NOTMUCH_STATUS_FILE_ERROR;
   goto DONE;
  }
  
  if (! S_ISDIR (st.st_mode)) {
 - fprintf (stderr, Error: Cannot create database at %s: Not a 
 directory.\n,
 -  path);
 + IGNORE_RESULT (asprintf (message, Error: Cannot create database at 
 %s: 
 +  Not a directory.\n,
 +  path));
   status = NOTMUCH_STATUS_FILE_ERROR;
   goto DONE;
  }
 @@ -640,15 +661,15 @@ notmuch_database_create (const char *path, 
 notmuch_database_t **database)
  err = mkdir (notmuch_path, 0755);
  
  if (err) {
 - fprintf (stderr, Error: Cannot create directory %s: %s.\n,
 -  notmuch_path, strerror (errno));
 + IGNORE_RESULT (asprintf (message, Error: Cannot create directory %s: 
 %s.\n,
 +  notmuch_path, strerror (errno)));
   status = NOTMUCH_STATUS_FILE_ERROR;
   goto DONE;
  }
  
 -status = notmuch_database_open (path,
 - NOTMUCH_DATABASE_MODE_READ_WRITE,
 - notmuch);
 +status = notmuch_database_open_verbose (path,
 + NOTMUCH_DATABASE_MODE_READ_WRITE,
 + notmuch, message);
  if (status)
   goto DONE;
  
 @@ -667,6 +688,9 @@ notmuch_database_create (const char *path, 
 notmuch_database_t **database)
  if (notmuch_path)
   talloc_free (notmuch_path);
  
 +if (status_string  message)
 + *status_string = message;

This series looks good and tests pass (I had confusion there before seeing
one of the last patch moving one output from stderr to stdout of where
the test status_cb prints it output)

Just that this small piece of code above git pass my sparse sieve, perhaps
this could be amended to:

if (message) {
if (status_string)
*status_string = message;
else 
free(message);
}


IMO either amend or leave to followup patch; just for that I don't
want to go through full patch series ;D


 +
  if (database)
   *database = notmuch;
  else
 @@ -767,37 +791,58 @@ notmuch_database_open (const char *path,
  notmuch_database_mode_t mode,
  notmuch_database_t **database)
  {
 +char *status_string = NULL;
 +notmuch_status_t status;
 +
 +status = notmuch_database_open_verbose (path, mode, database,
 +status_string);
 +
 +if (status_string) {
 + fputs (status_string, stderr);
 + free (status_string);
 +}
 +
 +return status;
 +}
 +

[Patch v6 4/8] lib: add verbose versions of notmuch_database_{open,create}

2015-03-27 Thread David Bremner 
The compatibility wrapper ensures that clients calling
notmuch_database_open will receive consistent output for now.

The changes to notmuch-{new,search} and test/symbol-test are just to
make the test suite pass.

The use of IGNORE_RESULT is justified by two things. 1) I don't know
what else to do.  2) asprintf guarantees the output string is NULL if
an error occurs, so at least we are not passing garbage back.
---
 lib/database.cc | 109 ++--
 lib/notmuch.h   |  21 ++
 notmuch-new.c   |  11 +-
 notmuch-search.c|  13 ++-
 test/symbol-test.cc |   9 -
 5 files changed, 130 insertions(+), 33 deletions(-)

diff --git a/lib/database.cc b/lib/database.cc
index 3974e2e..5d973b9 100644
--- a/lib/database.cc
+++ b/lib/database.cc
@@ -608,29 +608,50 @@ parse_references (void *ctx,
 notmuch_status_t
 notmuch_database_create (const char *path, notmuch_database_t **database)
 {
+char *status_string = NULL;
+notmuch_status_t status;
+
+status = notmuch_database_create_verbose (path, database,
+ status_string);
+
+if (status_string) {
+   fputs (status_string, stderr);
+   free (status_string);
+}
+
+return status;
+}
+
+notmuch_status_t
+notmuch_database_create_verbose (const char *path,
+notmuch_database_t **database,
+char **status_string)
+{
 notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 notmuch_database_t *notmuch = NULL;
 char *notmuch_path = NULL;
+char *message = NULL;
 struct stat st;
 int err;
 
 if (path == NULL) {
-   fprintf (stderr, Error: Cannot create a database for a NULL path.\n);
+   message = strdup (Error: Cannot create a database for a NULL path.\n);
status = NOTMUCH_STATUS_NULL_POINTER;
goto DONE;
 }
 
 err = stat (path, st);
 if (err) {
-   fprintf (stderr, Error: Cannot create database at %s: %s.\n,
-path, strerror (errno));
+   IGNORE_RESULT (asprintf (message, Error: Cannot create database at 
%s: %s.\n,
+   path, strerror (errno)));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
 
 if (! S_ISDIR (st.st_mode)) {
-   fprintf (stderr, Error: Cannot create database at %s: Not a 
directory.\n,
-path);
+   IGNORE_RESULT (asprintf (message, Error: Cannot create database at 
%s: 
+Not a directory.\n,
+path));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
@@ -640,15 +661,15 @@ notmuch_database_create (const char *path, 
notmuch_database_t **database)
 err = mkdir (notmuch_path, 0755);
 
 if (err) {
-   fprintf (stderr, Error: Cannot create directory %s: %s.\n,
-notmuch_path, strerror (errno));
+   IGNORE_RESULT (asprintf (message, Error: Cannot create directory %s: 
%s.\n,
+notmuch_path, strerror (errno)));
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
 }
 
-status = notmuch_database_open (path,
-   NOTMUCH_DATABASE_MODE_READ_WRITE,
-   notmuch);
+status = notmuch_database_open_verbose (path,
+   NOTMUCH_DATABASE_MODE_READ_WRITE,
+   notmuch, message);
 if (status)
goto DONE;
 
@@ -667,6 +688,9 @@ notmuch_database_create (const char *path, 
notmuch_database_t **database)
 if (notmuch_path)
talloc_free (notmuch_path);
 
+if (status_string  message)
+   *status_string = message;
+
 if (database)
*database = notmuch;
 else
@@ -767,37 +791,58 @@ notmuch_database_open (const char *path,
   notmuch_database_mode_t mode,
   notmuch_database_t **database)
 {
+char *status_string = NULL;
+notmuch_status_t status;
+
+status = notmuch_database_open_verbose (path, mode, database,
+  status_string);
+
+if (status_string) {
+   fputs (status_string, stderr);
+   free (status_string);
+}
+
+return status;
+}
+
+notmuch_status_t
+notmuch_database_open_verbose (const char *path,
+  notmuch_database_mode_t mode,
+  notmuch_database_t **database,
+  char **status_string)
+{
 notmuch_status_t status = NOTMUCH_STATUS_SUCCESS;
 void *local = talloc_new (NULL);
 notmuch_database_t *notmuch = NULL;
 char *notmuch_path, *xapian_path, *incompat_features;
+char *message = NULL;
 struct stat st;
 int err;
 unsigned int i, version;
 static int initialized = 0;
 
 if (path == NULL) {
-   fprintf (stderr, Error: Cannot open a database for a