* [PATCH] config: do not overwrite symlinks when saving config file
@ 2013-03-02 21:24 Jani Nikula
2013-03-02 21:58 ` Tomi Ollila
0 siblings, 1 reply; 5+ messages in thread
From: Jani Nikula @ 2013-03-02 21:24 UTC (permalink / raw)
To: notmuch
Use realpath on the config path before writing. If that fails,
fallback to the previous behaviour.
Previously 'notmuch setup' and 'notmuch config set' overwrote the
config file even if it was a symbolic link.
---
notmuch-config.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)
diff --git a/notmuch-config.c b/notmuch-config.c
index b5c2066..1e7389f 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -461,7 +461,7 @@ int
notmuch_config_save (notmuch_config_t *config)
{
size_t length;
- char *data;
+ char *data, *filename;
GError *error = NULL;
data = g_key_file_to_data (config->key_file, &length, NULL);
@@ -470,14 +470,20 @@ notmuch_config_save (notmuch_config_t *config)
return 1;
}
- if (! g_file_set_contents (config->filename, data, length, &error)) {
+ /* Try not to overwrite symlinks. */
+ filename = realpath (config->filename, NULL);
+
+ if (! g_file_set_contents (filename ? filename : config->filename,
+ data, length, &error)) {
fprintf (stderr, "Error saving configuration to %s: %s\n",
config->filename, error->message);
g_error_free (error);
+ free (filename);
g_free (data);
return 1;
}
+ free (filename);
g_free (data);
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH] config: do not overwrite symlinks when saving config file
2013-03-02 21:24 [PATCH] config: do not overwrite symlinks when saving config file Jani Nikula
@ 2013-03-02 21:58 ` Tomi Ollila
2013-03-03 9:32 ` [PATCH v2] " Jani Nikula
2013-03-03 14:18 ` [PATCH v3] " Jani Nikula
0 siblings, 2 replies; 5+ messages in thread
From: Tomi Ollila @ 2013-03-02 21:58 UTC (permalink / raw)
To: Jani Nikula, notmuch
On Sat, Mar 02 2013, Jani Nikula <jani@nikula.org> wrote:
> Use realpath on the config path before writing. If that fails,
> fallback to the previous behaviour.
>
> Previously 'notmuch setup' and 'notmuch config set' overwrote the
> config file even if it was a symbolic link.
> ---
> notmuch-config.c | 10 ++++++++--
> 1 file changed, 8 insertions(+), 2 deletions(-)
>
> diff --git a/notmuch-config.c b/notmuch-config.c
> index b5c2066..1e7389f 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -461,7 +461,7 @@ int
> notmuch_config_save (notmuch_config_t *config)
> {
> size_t length;
> - char *data;
> + char *data, *filename;
> GError *error = NULL;
>
> data = g_key_file_to_data (config->key_file, &length, NULL);
> @@ -470,14 +470,20 @@ notmuch_config_save (notmuch_config_t *config)
> return 1;
> }
>
> - if (! g_file_set_contents (config->filename, data, length, &error)) {
> + /* Try not to overwrite symlinks. */
> + filename = realpath (config->filename, NULL);
The code looks good to me. But should we handle realpath returning NULL
differently. It should return NULL only on error. I suggest that
when realpath returns NULL, descriptive enough error message is returned
to the user so that the problem can be resolved and then tried again.
Tomi
> +
> + if (! g_file_set_contents (filename ? filename : config->filename,
> + data, length, &error)) {
> fprintf (stderr, "Error saving configuration to %s: %s\n",
> config->filename, error->message);
> g_error_free (error);
> + free (filename);
> g_free (data);
> return 1;
> }
>
> + free (filename);
> g_free (data);
> return 0;
> }
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 5+ messages in thread
* [PATCH v2] config: do not overwrite symlinks when saving config file
2013-03-02 21:58 ` Tomi Ollila
@ 2013-03-03 9:32 ` Jani Nikula
2013-03-03 14:18 ` [PATCH v3] " Jani Nikula
1 sibling, 0 replies; 5+ messages in thread
From: Jani Nikula @ 2013-03-03 9:32 UTC (permalink / raw)
To: notmuch; +Cc: Tomi Ollila
Use realpath to canonicalize the config path before writing.
Previously 'notmuch setup' and 'notmuch config set' overwrote the
config file even if it was a symbolic link.
---
notmuch-config.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/notmuch-config.c b/notmuch-config.c
index b5c2066..89bc143 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -461,7 +461,7 @@ int
notmuch_config_save (notmuch_config_t *config)
{
size_t length;
- char *data;
+ char *data, *filename;
GError *error = NULL;
data = g_key_file_to_data (config->key_file, &length, NULL);
@@ -470,14 +470,30 @@ notmuch_config_save (notmuch_config_t *config)
return 1;
}
- if (! g_file_set_contents (config->filename, data, length, &error)) {
- fprintf (stderr, "Error saving configuration to %s: %s\n",
- config->filename, error->message);
+ /* Try not to overwrite symlinks. */
+ filename = realpath (config->filename, NULL);
+ if (! filename) {
+ fprintf (stderr, "Error canonicalizing %s: %s\n", config->filename,
+ strerror (errno));
+ g_free (data);
+ return 1;
+ }
+
+ if (! g_file_set_contents (filename, data, length, &error)) {
+ if (strcmp (filename, config->filename)) {
+ fprintf (stderr, "Error saving configuration to %s (-> %s): %s\n",
+ config->filename, filename, error->message);
+ } else {
+ fprintf (stderr, "Error saving configuration to %s: %s\n",
+ filename, error->message);
+ }
g_error_free (error);
+ free (filename);
g_free (data);
return 1;
}
+ free (filename);
g_free (data);
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* [PATCH v3] config: do not overwrite symlinks when saving config file
2013-03-02 21:58 ` Tomi Ollila
2013-03-03 9:32 ` [PATCH v2] " Jani Nikula
@ 2013-03-03 14:18 ` Jani Nikula
2013-03-03 18:38 ` Tomi Ollila
1 sibling, 1 reply; 5+ messages in thread
From: Jani Nikula @ 2013-03-03 14:18 UTC (permalink / raw)
To: notmuch; +Cc: Tomi Ollila
Use realpath to canonicalize the config path before writing.
Previously 'notmuch setup' and 'notmuch config set' overwrote the
config file even if it was a symbolic link.
---
v3: compared to v2, as suggested by Tomi on IRC:
- if (strcmp (filename, config->filename)) {
+ if (strcmp (filename, config->filename) != 0) {
---
notmuch-config.c | 24 ++++++++++++++++++++----
1 file changed, 20 insertions(+), 4 deletions(-)
diff --git a/notmuch-config.c b/notmuch-config.c
index b5c2066..8adb404 100644
--- a/notmuch-config.c
+++ b/notmuch-config.c
@@ -461,7 +461,7 @@ int
notmuch_config_save (notmuch_config_t *config)
{
size_t length;
- char *data;
+ char *data, *filename;
GError *error = NULL;
data = g_key_file_to_data (config->key_file, &length, NULL);
@@ -470,14 +470,30 @@ notmuch_config_save (notmuch_config_t *config)
return 1;
}
- if (! g_file_set_contents (config->filename, data, length, &error)) {
- fprintf (stderr, "Error saving configuration to %s: %s\n",
- config->filename, error->message);
+ /* Try not to overwrite symlinks. */
+ filename = realpath (config->filename, NULL);
+ if (! filename) {
+ fprintf (stderr, "Error canonicalizing %s: %s\n", config->filename,
+ strerror (errno));
+ g_free (data);
+ return 1;
+ }
+
+ if (! g_file_set_contents (filename, data, length, &error)) {
+ if (strcmp (filename, config->filename) != 0) {
+ fprintf (stderr, "Error saving configuration to %s (-> %s): %s\n",
+ config->filename, filename, error->message);
+ } else {
+ fprintf (stderr, "Error saving configuration to %s: %s\n",
+ filename, error->message);
+ }
g_error_free (error);
+ free (filename);
g_free (data);
return 1;
}
+ free (filename);
g_free (data);
return 0;
}
--
1.7.10.4
^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [PATCH v3] config: do not overwrite symlinks when saving config file
2013-03-03 14:18 ` [PATCH v3] " Jani Nikula
@ 2013-03-03 18:38 ` Tomi Ollila
0 siblings, 0 replies; 5+ messages in thread
From: Tomi Ollila @ 2013-03-03 18:38 UTC (permalink / raw)
To: Jani Nikula, notmuch
On Sun, Mar 03 2013, Jani Nikula <jani@nikula.org> wrote:
> Use realpath to canonicalize the config path before writing.
>
> Previously 'notmuch setup' and 'notmuch config set' overwrote the
> config file even if it was a symbolic link.
>
> ---
>
> v3: compared to v2, as suggested by Tomi on IRC:
>
> - if (strcmp (filename, config->filename)) {
> + if (strcmp (filename, config->filename) != 0) {
LGTM.
Tomi
> ---
> notmuch-config.c | 24 ++++++++++++++++++++----
> 1 file changed, 20 insertions(+), 4 deletions(-)
>
> diff --git a/notmuch-config.c b/notmuch-config.c
> index b5c2066..8adb404 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -461,7 +461,7 @@ int
> notmuch_config_save (notmuch_config_t *config)
> {
> size_t length;
> - char *data;
> + char *data, *filename;
> GError *error = NULL;
>
> data = g_key_file_to_data (config->key_file, &length, NULL);
> @@ -470,14 +470,30 @@ notmuch_config_save (notmuch_config_t *config)
> return 1;
> }
>
> - if (! g_file_set_contents (config->filename, data, length, &error)) {
> - fprintf (stderr, "Error saving configuration to %s: %s\n",
> - config->filename, error->message);
> + /* Try not to overwrite symlinks. */
> + filename = realpath (config->filename, NULL);
> + if (! filename) {
> + fprintf (stderr, "Error canonicalizing %s: %s\n", config->filename,
> + strerror (errno));
> + g_free (data);
> + return 1;
> + }
> +
> + if (! g_file_set_contents (filename, data, length, &error)) {
> + if (strcmp (filename, config->filename) != 0) {
> + fprintf (stderr, "Error saving configuration to %s (-> %s): %s\n",
> + config->filename, filename, error->message);
> + } else {
> + fprintf (stderr, "Error saving configuration to %s: %s\n",
> + filename, error->message);
> + }
> g_error_free (error);
> + free (filename);
> g_free (data);
> return 1;
> }
>
> + free (filename);
> g_free (data);
> return 0;
> }
> --
> 1.7.10.4
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2013-03-03 18:39 UTC | newest]
Thread overview: 5+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2013-03-02 21:24 [PATCH] config: do not overwrite symlinks when saving config file Jani Nikula
2013-03-02 21:58 ` Tomi Ollila
2013-03-03 9:32 ` [PATCH v2] " Jani Nikula
2013-03-03 14:18 ` [PATCH v3] " Jani Nikula
2013-03-03 18:38 ` 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).