From: Tomi Ollila <tomi.ollila@iki.fi>
To: Bijan Chokoufe Nejad <bijan@chokoufe.com>, notmuch@notmuchmail.org
Subject: Re: [PATCH] config: Expand ~ to $HOME
In-Reply-To: <1462722574-4176-1-git-send-email-bijan@chokoufe.com>
References: <1462722574-4176-1-git-send-email-bijan@chokoufe.com>
User-Agent: Notmuch/0.22+9~gb26b5ad (http://notmuchmail.org) Emacs/24.3.1
 (x86_64-unknown-linux-gnu)
Date: Sun, 08 May 2016 19:47:27 +0300
Message-ID: <m2r3dc7azk.fsf@guru.guru-group.fi>
MIME-Version: 1.0
Content-Type: text/plain
Precedence: list

On Sun, May 08 2016, Bijan Chokoufe Nejad <bijan@chokoufe.com> wrote:

> Very useful in case you want to keep your .notmuch-config synchronized across
> machines where you have different user names.

Thank you for your interest in improving notmuch!

There are a few things that needs to be sorted out for this feature to be
good:

This implementation does not handle ~user/ prefix: i.e. home directory of
'user' (maybe this should not, but it should handle the case).

Whether or not ~user is handled, it should check that slash (/) follows...

IIRC there is some ready-made implementations of the above -- but if not,
one option is to check how (expand-file-name) works in emacs for reference.


Something more inline:


> ---
> Not sure this is completely plattform independent.

The implementation looked like it is platform independent -- at least on
plattforms we care about...

> I also don't know how to implement a unit test for this.

I know... and I can do that if we get 1) decide that this feature will be
supported and 2) decide how this feature should work and 3) someone(tm)
does proper implementation ;)

Tomi

PS: still more to follow below.


> ---
>  notmuch-config.c | 11 ++++++++++-
>  1 file changed, 10 insertions(+), 1 deletion(-)
>
> diff --git a/notmuch-config.c b/notmuch-config.c
> index d252bb2..c9f26ef 100644
> --- a/notmuch-config.c
> +++ b/notmuch-config.c
> @@ -605,7 +605,16 @@ _config_set_list (notmuch_config_t *config,
>  const char *
>  notmuch_config_get_database_path (notmuch_config_t *config)
>  {
> -    return _config_get (config, &config->database_path, "database", "path");
> +    const char* path = _config_get (config, &config->database_path, "database", "path");
> +    if (path != NULL && path[0] == '~') {
> +        char *home_path = getenv("HOME");
> +        char *shortened_path = malloc( sizeof(char) * ( strlen (path) - 1 ) );
> +        strncpy(shortened_path, path + 2, strlen(path));
> +        return talloc_asprintf (NULL, "%s/%s", home_path, shortened_path);

In the implementation above matching free() for malloc() is not done -- but
actually the malloc is unnecessary -- path + 2 could have been used there
(after one checked that path[1] is '/' (provided that ~/ were the only
thing we supported...))

In strncpy() above length arg is strlen(path) but there is 1 byte less
allocated in shortened_path -- if src arg in the above strncpy() were
something else it could overwrite the allocated space by 2 bytes.



> +    }
> +    else {
> +        return path;
> +    }
>  }
>  
>  void
> -- 
> 1.9.1
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> https://notmuchmail.org/mailman/listinfo/notmuch