* [PATCH] notmuch-config: replace config reading function
@ 2016-11-06 0:09 Ioan-Adrian Ratiu
2016-11-06 9:30 ` Jani Nikula
0 siblings, 1 reply; 3+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-11-06 0:09 UTC (permalink / raw)
To: notmuch
Config files are currently read using glib's g_key_file_load_from_file
function which is very inconvenient because it's limited by design to read
only from "regular data files" in a filesystem. Because of this limitation
notmuch can't read configs from pipes, fifos, sockets, stdin, etc. Not even
"notmuch --config=/dev/stdin" works:
Error reading configuration file /dev/stdin: Not a regular file
So replace g_key_file_load_from_file with g_key_file_load_from_data which
gives us much more freedom to read configs from multiple sources.
This also helps the more security sensitive users: If someone has private
information in the config file, it can be encrypted on disk, then decrypted
in RAM and passed through a pipe directly to notmuch without the use of
intermediate plain text files.
Signed-off-by: Ioan-Adrian Ratiu <adi@adirat.com>
---
notmuch-config.c | 63 ++++++++++++++++++++++++++++++++++++++++++++++++++++----
1 file changed, 59 insertions(+), 4 deletions(-)
diff --git a/notmuch-config.c b/notmuch-config.c
index e5d42a0..8435815 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -202,6 +202,64 @@ get_username_from_passwd_file (void *ctx)
return name;
}
+static gboolean
+get_config_from_file (notmuch_config_t *config, GError **error)
+{
+ #define BUF_SIZE 4096
+ char buffer[BUF_SIZE];
+ size_t content_size = 1; // includes NULL
+
+ FILE *fp = fopen(config->filename, "r");
+ if (fp == NULL) {
+ *error = g_error_new(G_FILE_ERROR,
+ G_FILE_ERROR_NOENT,
+ "Couldn't open config file '%s': %s.\n",
+ config->filename,
+ strerror(errno));
+ return FALSE;
+ }
+
+ char *config_str = talloc_zero_array (config, char, BUF_SIZE);
+ if (config_str == NULL)
+ {
+ *error = g_error_new(G_FILE_ERROR,
+ G_FILE_ERROR_NOMEM,
+ "Out of memory while reading config.\n");
+ return FALSE;
+ }
+
+ while (fgets (buffer, BUF_SIZE, fp))
+ {
+ content_size += strlen(buffer);
+ config_str = talloc_realloc(config, config_str, char, content_size);
+ if (config_str == NULL)
+ {
+ *error = g_error_new(G_FILE_ERROR,
+ G_FILE_ERROR_NOMEM,
+ "Failed to reallocate config memory.\n");
+ return FALSE;
+ }
+ strcat (config_str, buffer);
+ }
+
+ if (ferror (fp))
+ {
+ *error = g_error_new(G_FILE_ERROR,
+ G_FILE_ERROR_IO,
+ "I/O error reading configuration from '%s'.\n",
+ config->filename);
+ return FALSE;
+ }
+
+ fclose(fp);
+
+ return g_key_file_load_from_data (config->key_file,
+ config_str,
+ strlen(config_str),
+ G_KEY_FILE_KEEP_COMMENTS,
+ error);
+}
+
/* Open the named notmuch configuration file. If the filename is NULL,
* the value of the environment variable $NOTMUCH_CONFIG will be used.
* If $NOTMUCH_CONFIG is unset, the default configuration file
@@ -289,10 +347,7 @@ notmuch_config_open (void *ctx,
config->search_exclude_tags_length = 0;
config->crypto_gpg_path = NULL;
- if (! g_key_file_load_from_file (config->key_file,
- config->filename,
- G_KEY_FILE_KEEP_COMMENTS,
- &error))
+ if (! get_config_from_file (config, &error))
{
if (error->domain == G_FILE_ERROR && error->code == G_FILE_ERROR_NOENT) {
/* If create_new is true, then the caller is prepared for a
--
2.10.2
^ permalink raw reply related [flat|nested] 3+ messages in thread
* Re: [PATCH] notmuch-config: replace config reading function
2016-11-06 0:09 [PATCH] notmuch-config: replace config reading function Ioan-Adrian Ratiu
@ 2016-11-06 9:30 ` Jani Nikula
2016-11-06 9:30 ` [PATCH] cli: abstract config file reading to a separate function Jani Nikula
0 siblings, 1 reply; 3+ messages in thread
From: Jani Nikula @ 2016-11-06 9:30 UTC (permalink / raw)
To: Ioan-Adrian Ratiu, notmuch
> So replace g_key_file_load_from_file with g_key_file_load_from_data which
> gives us much more freedom to read configs from multiple sources.
I see how your function is a drop-in replacement to
g_key_file_load_from_file. However, I really don't like the
proliferation of GError here. Please consider this prep patch first. The
error handling will be easier to follow.
BR,
Jani.
Jani Nikula (1):
cli: abstract config file reading to a separate function
notmuch-config.c | 65 ++++++++++++++++++++++++++++++--------------------------
1 file changed, 35 insertions(+), 30 deletions(-)
--
2.1.4
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] cli: abstract config file reading to a separate function
2016-11-06 9:30 ` Jani Nikula
@ 2016-11-06 9:30 ` Jani Nikula
0 siblings, 0 replies; 3+ messages in thread
From: Jani Nikula @ 2016-11-06 9:30 UTC (permalink / raw)
To: Ioan-Adrian Ratiu, notmuch
Simplify and fix the coding style while at it.
---
notmuch-config.c | 65 ++++++++++++++++++++++++++++++--------------------------
1 file changed, 35 insertions(+), 30 deletions(-)
diff --git a/notmuch-config.c b/notmuch-config.c
index e5d42a0cbfd5..bd527901f7b5 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -202,6 +202,38 @@ get_username_from_passwd_file (void *ctx)
return name;
}
+static notmuch_bool_t
+get_config_from_file (notmuch_config_t *config, notmuch_bool_t create_new)
+{
+ GError *error = NULL;
+ notmuch_bool_t ret = FALSE;
+
+ if (g_key_file_load_from_file (config->key_file, config->filename,
+ G_KEY_FILE_KEEP_COMMENTS, &error))
+ return TRUE;
+
+ if (error->domain == G_FILE_ERROR && error->code == G_FILE_ERROR_NOENT) {
+ /* If create_new is true, then the caller is prepared for a
+ * default configuration file in the case of FILE NOT FOUND.
+ */
+ if (create_new) {
+ config->is_new = TRUE;
+ ret = TRUE;
+ } else {
+ fprintf (stderr, "Configuration file %s not found.\n"
+ "Try running 'notmuch setup' to create a configuration.\n",
+ config->filename);
+ }
+ } else {
+ fprintf (stderr, "Error reading configuration file %s: %s\n",
+ config->filename, error->message);
+ }
+
+ g_error_free (error);
+
+ return ret;
+}
+
/* Open the named notmuch configuration file. If the filename is NULL,
* the value of the environment variable $NOTMUCH_CONFIG will be used.
* If $NOTMUCH_CONFIG is unset, the default configuration file
@@ -289,36 +321,9 @@ notmuch_config_open (void *ctx,
config->search_exclude_tags_length = 0;
config->crypto_gpg_path = NULL;
- if (! g_key_file_load_from_file (config->key_file,
- config->filename,
- G_KEY_FILE_KEEP_COMMENTS,
- &error))
- {
- if (error->domain == G_FILE_ERROR && error->code == G_FILE_ERROR_NOENT) {
- /* If create_new is true, then the caller is prepared for a
- * default configuration file in the case of FILE NOT
- * FOUND.
- */
- if (create_new) {
- g_error_free (error);
- config->is_new = TRUE;
- } else {
- fprintf (stderr, "Configuration file %s not found.\n"
- "Try running 'notmuch setup' to create a configuration.\n",
- config->filename);
- talloc_free (config);
- g_error_free (error);
- return NULL;
- }
- }
- else
- {
- fprintf (stderr, "Error reading configuration file %s: %s\n",
- config->filename, error->message);
- talloc_free (config);
- g_error_free (error);
- return NULL;
- }
+ if (! get_config_from_file (config, create_new)) {
+ talloc_free (config);
+ return NULL;
}
/* Whenever we know of configuration sections that don't appear in
--
2.1.4
^ permalink raw reply related [flat|nested] 3+ messages in thread
end of thread, other threads:[~2016-11-06 9:32 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-06 0:09 [PATCH] notmuch-config: replace config reading function Ioan-Adrian Ratiu
2016-11-06 9:30 ` Jani Nikula
2016-11-06 9:30 ` [PATCH] cli: abstract config file reading to a separate function Jani Nikula
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).