* [PATCH v2 0/2] Refactor config reading to support non-regular files
@ 2016-11-06 13:55 Ioan-Adrian Ratiu
2016-11-06 13:55 ` [PATCH v2 1/2] cli: abstract config file reading to a separate function Ioan-Adrian Ratiu
2016-11-06 13:55 ` [PATCH v2 2/2] notmuch-config: replace config reading function Ioan-Adrian Ratiu
0 siblings, 2 replies; 4+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-11-06 13:55 UTC (permalink / raw)
To: jani, notmuch
Changes since v1 (Based on Jani's feedback):
* Incorporated Jani's patch into this series and rebased my changes on top
Ioan-Adrian Ratiu (1):
notmuch-config: replace config reading function
Jani Nikula (1):
cli: abstract config file reading to a separate function
notmuch-config.c | 94 ++++++++++++++++++++++++++++++++++++++------------------
1 file changed, 64 insertions(+), 30 deletions(-)
--
2.10.2
^ permalink raw reply [flat|nested] 4+ messages in thread
* [PATCH v2 1/2] cli: abstract config file reading to a separate function
2016-11-06 13:55 [PATCH v2 0/2] Refactor config reading to support non-regular files Ioan-Adrian Ratiu
@ 2016-11-06 13:55 ` Ioan-Adrian Ratiu
2016-11-06 13:55 ` [PATCH v2 2/2] notmuch-config: replace config reading function Ioan-Adrian Ratiu
1 sibling, 0 replies; 4+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-11-06 13:55 UTC (permalink / raw)
To: jani, notmuch
From: Jani Nikula <jani@nikula.org>
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 e5d42a0..bd52790 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.10.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* [PATCH v2 2/2] notmuch-config: replace config reading function
2016-11-06 13:55 [PATCH v2 0/2] Refactor config reading to support non-regular files Ioan-Adrian Ratiu
2016-11-06 13:55 ` [PATCH v2 1/2] cli: abstract config file reading to a separate function Ioan-Adrian Ratiu
@ 2016-11-06 13:55 ` Ioan-Adrian Ratiu
2016-12-04 13:36 ` Tomi Ollila
1 sibling, 1 reply; 4+ messages in thread
From: Ioan-Adrian Ratiu @ 2016-11-06 13:55 UTC (permalink / raw)
To: jani, 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 | 53 +++++++++++++++++++++++++++++++++++++++++------------
1 file changed, 41 insertions(+), 12 deletions(-)
diff --git a/notmuch-config.c b/notmuch-config.c
index bd52790..569cf0b 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -205,33 +205,62 @@ get_username_from_passwd_file (void *ctx)
static notmuch_bool_t
get_config_from_file (notmuch_config_t *config, notmuch_bool_t create_new)
{
+ #define BUF_SIZE 4096
+ char buffer[BUF_SIZE];
+ size_t content_size = 1; // includes NULL
+ char *config_str = NULL;
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) {
+ FILE *fp = fopen(config->filename, "r");
+ if (fp == NULL) {
/* 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;
+ return TRUE;
} else {
- fprintf (stderr, "Configuration file %s not found.\n"
+ fprintf (stderr, "Error opening config file '%s': %s\n"
"Try running 'notmuch setup' to create a configuration.\n",
+ config->filename, strerror(errno));
+ return FALSE;
+ }
+ }
+
+ config_str = talloc_zero_array (config, char, BUF_SIZE);
+ if (config_str == NULL) {
+ fprintf (stderr, "Error reading '%s': Out of memory\n", config->filename);
+ 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) {
+ fprintf (stderr, "Error reading '%s': Failed to reallocate memory\n",
config->filename);
+ return FALSE;
}
- } else {
- fprintf (stderr, "Error reading configuration file %s: %s\n",
- config->filename, error->message);
+ strcat (config_str, buffer);
+ }
+
+ if (ferror (fp)) {
+ fprintf (stderr, "Error reading '%s': I/O error\n", config->filename);
+ return FALSE;
}
+ fclose(fp);
+
+ if (g_key_file_load_from_data (config->key_file, config_str, strlen(config_str),
+ G_KEY_FILE_KEEP_COMMENTS, &error))
+ return TRUE;
+
+ fprintf (stderr, "Error parsing config file '%s': %s\n",
+ config->filename, error->message);
+
g_error_free (error);
- return ret;
+ return FALSE;
}
/* Open the named notmuch configuration file. If the filename is NULL,
--
2.10.2
^ permalink raw reply related [flat|nested] 4+ messages in thread
* Re: [PATCH v2 2/2] notmuch-config: replace config reading function
2016-11-06 13:55 ` [PATCH v2 2/2] notmuch-config: replace config reading function Ioan-Adrian Ratiu
@ 2016-12-04 13:36 ` Tomi Ollila
0 siblings, 0 replies; 4+ messages in thread
From: Tomi Ollila @ 2016-12-04 13:36 UTC (permalink / raw)
To: Ioan-Adrian Ratiu, notmuch
On Sun, Nov 06 2016, Ioan-Adrian Ratiu <adi@adirat.com> wrote:
> 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>
I'd suggest the reading loop be something like (error handling (mostly) missing,
untested):
// #define BUF_SIZE 200 // test line (use debug code or strace(1) to verify)
#define BUF_SIZE 4096
char *config_str;
int config_len = 0;
int config_bufsize = BUF_SIZE;
size_t len;
...
FILE *fp = fopen (config->filename, "r");
...
config_str = talloc_zero_array (config, char, config_bufsize);
...
while ((len = fread (config_str + config_len, 1,
config_bufsize - config_len, fp)) > 0) {
config_len += len;
if (config_len == config_bufsize) {
config_bufsize += BUF_SIZE;
config_str = talloc_realloc (config, config_str, char, config_bufsize);
}
}
if (ferror (fp)) {
fprintf (stderr, "Error reading '%s': I/O error\n", config->filename);
fclose (fp);
return FALSE;
}
fclose (fp)
if (g_key_file_load_from_data (config->key_file, config_str, config_len,
G_KEY_FILE_KEEP_COMMENTS, &error))
return TRUE;
> ---
> notmuch-config.c | 53 +++++++++++++++++++++++++++++++++++++++++------------
> 1 file changed, 41 insertions(+), 12 deletions(-)
>
> diff --git a/notmuch-config.c b/notmuch-config.c
> index bd52790..569cf0b 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -205,33 +205,62 @@ get_username_from_passwd_file (void *ctx)
> static notmuch_bool_t
> get_config_from_file (notmuch_config_t *config, notmuch_bool_t create_new)
> {
> + #define BUF_SIZE 4096
> + char buffer[BUF_SIZE];
> + size_t content_size = 1; // includes NULL
> + char *config_str = NULL;
> 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) {
> + FILE *fp = fopen(config->filename, "r");
> + if (fp == NULL) {
> /* 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;
> + return TRUE;
> } else {
> - fprintf (stderr, "Configuration file %s not found.\n"
> + fprintf (stderr, "Error opening config file '%s': %s\n"
> "Try running 'notmuch setup' to create a configuration.\n",
> + config->filename, strerror(errno));
> + return FALSE;
> + }
> + }
> +
> + config_str = talloc_zero_array (config, char, BUF_SIZE);
> + if (config_str == NULL) {
> + fprintf (stderr, "Error reading '%s': Out of memory\n", config->filename);
> + 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) {
> + fprintf (stderr, "Error reading '%s': Failed to reallocate memory\n",
> config->filename);
> + return FALSE;
> }
> - } else {
> - fprintf (stderr, "Error reading configuration file %s: %s\n",
> - config->filename, error->message);
> + strcat (config_str, buffer);
> + }
> +
> + if (ferror (fp)) {
> + fprintf (stderr, "Error reading '%s': I/O error\n", config->filename);
> + return FALSE;
> }
>
> + fclose(fp);
> +
> + if (g_key_file_load_from_data (config->key_file, config_str, strlen(config_str),
> + G_KEY_FILE_KEEP_COMMENTS, &error))
> + return TRUE;
> +
> + fprintf (stderr, "Error parsing config file '%s': %s\n",
> + config->filename, error->message);
> +
> g_error_free (error);
>
> - return ret;
> + return FALSE;
> }
>
> /* Open the named notmuch configuration file. If the filename is NULL,
> --
> 2.10.2
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2016-12-04 13:35 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2016-11-06 13:55 [PATCH v2 0/2] Refactor config reading to support non-regular files Ioan-Adrian Ratiu
2016-11-06 13:55 ` [PATCH v2 1/2] cli: abstract config file reading to a separate function Ioan-Adrian Ratiu
2016-11-06 13:55 ` [PATCH v2 2/2] notmuch-config: replace config reading function Ioan-Adrian Ratiu
2016-12-04 13:36 ` Tomi Ollila
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).