* WIP: report errors from GLib INI parser
@ 2023-09-13 0:56 David Bremner
2023-09-13 0:56 ` [PATCH 1/3] test: add known broken test for bad utf8 in config David Bremner
` (2 more replies)
0 siblings, 3 replies; 6+ messages in thread
From: David Bremner @ 2023-09-13 0:56 UTC (permalink / raw)
To: notmuch; +Cc: eblake
I'm not too happy with the last patch, but it (in addition to
borrowing heavily from Eric's previous patch without credit) shows the
general idea of what has to be done. I'm not sure, but perhaps the
internal API _notmuch_config_load_from_file should take a status
string argument in the style of several of the other utility functions
used in this part of the code.
The last patch also needs a test to verify the message is being
printed, but I'm out of time for the moment.
^ permalink raw reply [flat|nested] 6+ messages in thread
* [PATCH 1/3] test: add known broken test for bad utf8 in config
2023-09-13 0:56 WIP: report errors from GLib INI parser David Bremner
@ 2023-09-13 0:56 ` David Bremner
2023-09-13 0:56 ` [PATCH 2/3] CLI: exit with error when load_config returns and error David Bremner
2023-09-13 0:56 ` [PATCH 3/3] WIP: print error message from glib ini parser David Bremner
2 siblings, 0 replies; 6+ messages in thread
From: David Bremner @ 2023-09-13 0:56 UTC (permalink / raw)
To: notmuch; +Cc: eblake
We should ideally print an informative error message, but at the very
least we should not exit with success.
---
test/T030-config.sh | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/test/T030-config.sh b/test/T030-config.sh
index ea0b4012..6ef759a4 100755
--- a/test/T030-config.sh
+++ b/test/T030-config.sh
@@ -3,6 +3,8 @@
test_description='"notmuch config"'
. $(dirname "$0")/test-lib.sh || exit 1
+cp notmuch-config initial-config
+
test_begin_subtest "Get string value"
test_expect_equal "$(notmuch config get user.name)" "Notmuch Test Suite"
@@ -193,4 +195,11 @@ test_begin_subtest "get built_with.nonexistent prints false"
output=$(notmuch config get built_with.nonexistent)
test_expect_equal "$output" "false"
+test_begin_subtest "Bad utf8 reported as error"
+test_subtest_known_broken
+cp initial-config bad-config
+printf '[query]\nq3=from:\xff\n' >>bad-config
+test_expect_code 1 "notmuch --config=./bad-config config list"
+
test_done
+
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 2/3] CLI: exit with error when load_config returns and error.
2023-09-13 0:56 WIP: report errors from GLib INI parser David Bremner
2023-09-13 0:56 ` [PATCH 1/3] test: add known broken test for bad utf8 in config David Bremner
@ 2023-09-13 0:56 ` David Bremner
2023-09-13 13:37 ` Eric Blake
2023-09-13 0:56 ` [PATCH 3/3] WIP: print error message from glib ini parser David Bremner
2 siblings, 1 reply; 6+ messages in thread
From: David Bremner @ 2023-09-13 0:56 UTC (permalink / raw)
To: notmuch; +Cc: eblake
---
notmuch.c | 2 ++
test/T030-config.sh | 1 -
2 files changed, 2 insertions(+), 1 deletion(-)
diff --git a/notmuch.c b/notmuch.c
index 37286b8f..43554530 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -584,6 +584,8 @@ main (int argc, char *argv[])
case NOTMUCH_STATUS_SUCCESS:
break;
default:
+ fputs("error loading config file\n", stderr);
+ ret = 1;
goto DONE;
}
diff --git a/test/T030-config.sh b/test/T030-config.sh
index 6ef759a4..8b3ef436 100755
--- a/test/T030-config.sh
+++ b/test/T030-config.sh
@@ -196,7 +196,6 @@ output=$(notmuch config get built_with.nonexistent)
test_expect_equal "$output" "false"
test_begin_subtest "Bad utf8 reported as error"
-test_subtest_known_broken
cp initial-config bad-config
printf '[query]\nq3=from:\xff\n' >>bad-config
test_expect_code 1 "notmuch --config=./bad-config config list"
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* [PATCH 3/3] WIP: print error message from glib ini parser
2023-09-13 0:56 WIP: report errors from GLib INI parser David Bremner
2023-09-13 0:56 ` [PATCH 1/3] test: add known broken test for bad utf8 in config David Bremner
2023-09-13 0:56 ` [PATCH 2/3] CLI: exit with error when load_config returns and error David Bremner
@ 2023-09-13 0:56 ` David Bremner
2023-09-13 13:41 ` Eric Blake
2 siblings, 1 reply; 6+ messages in thread
From: David Bremner @ 2023-09-13 0:56 UTC (permalink / raw)
To: notmuch; +Cc: eblake
---
lib/config.cc | 7 ++++++-
lib/open.cc | 4 ++++
notmuch.c | 6 ++++++
3 files changed, 16 insertions(+), 1 deletion(-)
diff --git a/lib/config.cc b/lib/config.cc
index 2323860d..11afb785 100644
--- a/lib/config.cc
+++ b/lib/config.cc
@@ -435,6 +435,7 @@ _notmuch_config_load_from_file (notmuch_database_t *notmuch,
for (gchar **keys_p = keys; *keys_p; keys_p++) {
char *absolute_key = talloc_asprintf (notmuch, "%s.%s", *grp, *keys_p);
char *normalized_val;
+ GError *gerr = NULL;
/* If we opened from a given path, do not overwrite it */
if (strcmp (absolute_key, "database.path") == 0 &&
@@ -442,7 +443,11 @@ _notmuch_config_load_from_file (notmuch_database_t *notmuch,
notmuch->xapian_db)
continue;
- val = g_key_file_get_string (file, *grp, *keys_p, NULL);
+ val = g_key_file_get_string (file, *grp, *keys_p, &gerr);
+ if (gerr) {
+ _notmuch_database_log (notmuch, "%s\n", gerr->message);
+ g_error_free (gerr);
+ }
if (! val) {
status = NOTMUCH_STATUS_FILE_ERROR;
goto DONE;
diff --git a/lib/open.cc b/lib/open.cc
index 54d1faf3..6dec7b07 100644
--- a/lib/open.cc
+++ b/lib/open.cc
@@ -556,6 +556,8 @@ _finish_open (notmuch_database_t *notmuch,
if (key_file)
status = _notmuch_config_load_from_file (notmuch, key_file);
+ if (notmuch_database_status_string (notmuch))
+ message = strdup( notmuch_database_status_string (notmuch));
if (status)
goto DONE;
@@ -962,6 +964,8 @@ notmuch_database_load_config (const char *database_path,
if (key_file) {
status = _notmuch_config_load_from_file (notmuch, key_file);
+ if (notmuch_database_status_string (notmuch))
+ message = strdup( notmuch_database_status_string (notmuch));
if (status)
goto DONE;
}
diff --git a/notmuch.c b/notmuch.c
index 43554530..e790a44b 100644
--- a/notmuch.c
+++ b/notmuch.c
@@ -563,6 +563,12 @@ main (int argc, char *argv[])
NULL,
¬much,
&status_string);
+ if (status_string) {
+ fputs (status_string, stderr);
+ free (status_string);
+ status_string = NULL;
+ }
+
switch (status) {
case NOTMUCH_STATUS_NO_CONFIG:
if (! (command->mode & NOTMUCH_COMMAND_CONFIG_CREATE)) {
--
2.40.1
^ permalink raw reply related [flat|nested] 6+ messages in thread
* Re: [PATCH 2/3] CLI: exit with error when load_config returns and error.
2023-09-13 0:56 ` [PATCH 2/3] CLI: exit with error when load_config returns and error David Bremner
@ 2023-09-13 13:37 ` Eric Blake
0 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2023-09-13 13:37 UTC (permalink / raw)
To: David Bremner; +Cc: notmuch
On Tue, Sep 12, 2023 at 09:56:35PM -0300, David Bremner wrote:
In the subject: s/and/an/
> ---
> notmuch.c | 2 ++
> test/T030-config.sh | 1 -
> 2 files changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/notmuch.c b/notmuch.c
> index 37286b8f..43554530 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -584,6 +584,8 @@ main (int argc, char *argv[])
> case NOTMUCH_STATUS_SUCCESS:
> break;
> default:
> + fputs("error loading config file\n", stderr);
> + ret = 1;
> goto DONE;
> }
>
> diff --git a/test/T030-config.sh b/test/T030-config.sh
> index 6ef759a4..8b3ef436 100755
> --- a/test/T030-config.sh
> +++ b/test/T030-config.sh
> @@ -196,7 +196,6 @@ output=$(notmuch config get built_with.nonexistent)
> test_expect_equal "$output" "false"
>
> test_begin_subtest "Bad utf8 reported as error"
> -test_subtest_known_broken
> cp initial-config bad-config
> printf '[query]\nq3=from:\xff\n' >>bad-config
> test_expect_code 1 "notmuch --config=./bad-config config list"
Definite progress! It doesn't say what the precise error is, but
calling out the config file as the problem is far better than silence.
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 6+ messages in thread
* Re: [PATCH 3/3] WIP: print error message from glib ini parser
2023-09-13 0:56 ` [PATCH 3/3] WIP: print error message from glib ini parser David Bremner
@ 2023-09-13 13:41 ` Eric Blake
0 siblings, 0 replies; 6+ messages in thread
From: Eric Blake @ 2023-09-13 13:41 UTC (permalink / raw)
To: David Bremner; +Cc: notmuch
On Tue, Sep 12, 2023 at 09:56:36PM -0300, David Bremner wrote:
> ---
> lib/config.cc | 7 ++++++-
> lib/open.cc | 4 ++++
> notmuch.c | 6 ++++++
> 3 files changed, 16 insertions(+), 1 deletion(-)
>
> diff --git a/lib/config.cc b/lib/config.cc
> index 2323860d..11afb785 100644
> --- a/lib/config.cc
> +++ b/lib/config.cc
> @@ -435,6 +435,7 @@ _notmuch_config_load_from_file (notmuch_database_t *notmuch,
> for (gchar **keys_p = keys; *keys_p; keys_p++) {
> char *absolute_key = talloc_asprintf (notmuch, "%s.%s", *grp, *keys_p);
> char *normalized_val;
> + GError *gerr = NULL;
>
> /* If we opened from a given path, do not overwrite it */
> if (strcmp (absolute_key, "database.path") == 0 &&
> @@ -442,7 +443,11 @@ _notmuch_config_load_from_file (notmuch_database_t *notmuch,
> notmuch->xapian_db)
> continue;
>
> - val = g_key_file_get_string (file, *grp, *keys_p, NULL);
> + val = g_key_file_get_string (file, *grp, *keys_p, &gerr);
> + if (gerr) {
> + _notmuch_database_log (notmuch, "%s\n", gerr->message);
> + g_error_free (gerr);
> + }
> if (! val) {
> status = NOTMUCH_STATUS_FILE_ERROR;
> goto DONE;
> diff --git a/lib/open.cc b/lib/open.cc
> index 54d1faf3..6dec7b07 100644
> --- a/lib/open.cc
> +++ b/lib/open.cc
> @@ -556,6 +556,8 @@ _finish_open (notmuch_database_t *notmuch,
>
> if (key_file)
> status = _notmuch_config_load_from_file (notmuch, key_file);
> + if (notmuch_database_status_string (notmuch))
> + message = strdup( notmuch_database_status_string (notmuch));
Unusual formatting; did you mean 'strdup (notmuch...'?
> if (status)
> goto DONE;
>
> @@ -962,6 +964,8 @@ notmuch_database_load_config (const char *database_path,
>
> if (key_file) {
> status = _notmuch_config_load_from_file (notmuch, key_file);
> + if (notmuch_database_status_string (notmuch))
> + message = strdup( notmuch_database_status_string (notmuch));
...and a copy/paste.
> if (status)
> goto DONE;
> }
> diff --git a/notmuch.c b/notmuch.c
> index 43554530..e790a44b 100644
> --- a/notmuch.c
> +++ b/notmuch.c
> @@ -563,6 +563,12 @@ main (int argc, char *argv[])
> NULL,
> ¬much,
> &status_string);
> + if (status_string) {
> + fputs (status_string, stderr);
> + free (status_string);
> + status_string = NULL;
> + }
> +
> switch (status) {
> case NOTMUCH_STATUS_NO_CONFIG:
> if (! (command->mode & NOTMUCH_COMMAND_CONFIG_CREATE)) {
> --
> 2.40.1
>
--
Eric Blake, Principal Software Engineer
Red Hat, Inc.
Virtualization: qemu.org | libguestfs.org
^ permalink raw reply [flat|nested] 6+ messages in thread
end of thread, other threads:[~2023-09-13 13:49 UTC | newest]
Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2023-09-13 0:56 WIP: report errors from GLib INI parser David Bremner
2023-09-13 0:56 ` [PATCH 1/3] test: add known broken test for bad utf8 in config David Bremner
2023-09-13 0:56 ` [PATCH 2/3] CLI: exit with error when load_config returns and error David Bremner
2023-09-13 13:37 ` Eric Blake
2023-09-13 0:56 ` [PATCH 3/3] WIP: print error message from glib ini parser David Bremner
2023-09-13 13:41 ` Eric Blake
Code repositories for project(s) associated with this public inbox
https://yhetil.org/notmuch.git/
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).