On 2021-04-18 10:08:31+0300, Tomi Ollila <tomi.oll...@iki.fi> wrote: > On Sun, Apr 18 2021, Đoàn Trần Công Danh wrote: > > > On 2021-04-17 11:39:59-0300, David Bremner <da...@tethera.net> wrote: > >> Đoàn Trần Công Danh <congdan...@gmail.com> writes: > >> > >> > > >> > However, I see that lib/open.cc uses g_key_file_get_value from GLib > >> > already, we may switch to g_canonicalize_file_name then? > >> > > >> > >> Yes that could work. I think the treatment of NULL input might need some > >> extra care with g_canonicalize_file_name; at least my 5 minute attempt > >> to do a replacement causes many test failures. Do you want to make a > >> patch that uses g_canonicalize_file_name? The one other place we use > >> canonicalize_file_name (not totally coincidentally) also uses glib, so > >> in principle we could completely eliminate the compat function. > > > > Now, I'm thinking more about it and digging into GLib history, > > I think using g_canonicalize_filename would be problem, since it's > > available from 2.57.1/2.58 only. I think we're requiring glib-2.0 2.22 > > as of it's now. A big jump in dependencies isn't worth it. > > > > I think below patch would be better? > > If that worked (I did not test), instead of macro "hackery", inline function > could be better(?) i.e. something like the following in notmuch_compat.h: > > #if HAVE_CANONICALIZE_FILE_NAME > static inline char * > notmuch_canonicalize_file_name (const char *path) > { > return canonicalize_file_name (path) > } > #else > char * > notmuch_canonicalize_file_name (const char *path); > #endif >
Yes, inline function are always better than macro. I feel embarassed that I couldn't think about that earlier. Here is a revised patch: ---8<---- Subject: [PATCH] compat: rename {,notmuch_}canonicalize_file_name When compat canonicalize_file_name was introduced, it was limited to C code only because it was used by C code only during that time. From 5ec6fd4d, (lib/open: check for split configuration when creating database., 2021-02-16), lib/open.cc, which is C++, relies on the existent of canonicalize_file_name. However, we can't blindly enable canonicalize_file_name for C++ code, because different implementation has different additional signature for C++ and users can arbitrarily add -DHAVE_CANONICALIZE_FILE_NAME=0 to {C,CXX}FLAGS. Let's put our implementation of canonicalize_file_name to our namespace, and prefer system's canonicalize_file_name if it's existed via an inline function. Helped-by: Tomi Ollila <tomi.oll...@iki.fi> --- compat/canonicalize_file_name.c | 2 +- compat/compat.h | 13 ++++++++----- lib/open.cc | 4 ++-- notmuch-config.c | 2 +- 4 files changed, 12 insertions(+), 9 deletions(-) diff --git a/compat/canonicalize_file_name.c b/compat/canonicalize_file_name.c index 000f9e78..84f2a2c2 100644 --- a/compat/canonicalize_file_name.c +++ b/compat/canonicalize_file_name.c @@ -4,7 +4,7 @@ #include <stdlib.h> char * -canonicalize_file_name (const char *path) +notmuch_canonicalize_file_name (const char *path) { #ifdef PATH_MAX char *resolved_path = malloc (PATH_MAX + 1); diff --git a/compat/compat.h b/compat/compat.h index 8f15e585..c3ef4051 100644 --- a/compat/compat.h +++ b/compat/compat.h @@ -37,12 +37,15 @@ extern "C" { #define _POSIX_PTHREAD_SEMANTICS 1 #endif -#if ! HAVE_CANONICALIZE_FILE_NAME -/* we only call this function from C, and this makes testing easier */ -#ifndef __cplusplus +#if HAVE_CANONICALIZE_FILE_NAME +static inline char * +notmuch_canonicalize_file_name (const char *path) +{ + return canonicalize_file_name (path); +} +#else char * -canonicalize_file_name (const char *path); -#endif +notmuch_canonicalize_file_name (const char *path); #endif #if ! HAVE_GETLINE diff --git a/lib/open.cc b/lib/open.cc index 5d80a884..7454ffae 100644 --- a/lib/open.cc +++ b/lib/open.cc @@ -612,9 +612,9 @@ notmuch_database_create_with_config (const char *database_path, _set_database_path (notmuch, database_path); if (key_file && ! split) { - char *mail_root = canonicalize_file_name ( + char *mail_root = notmuch_canonicalize_file_name ( g_key_file_get_value (key_file, "database", "mail_root", NULL)); - char *db_path = canonicalize_file_name (database_path); + char *db_path = notmuch_canonicalize_file_name (database_path); split = (mail_root && (0 != strcmp (mail_root, db_path))); diff --git a/notmuch-config.c b/notmuch-config.c index 16e86916..d8d47768 100644 --- a/notmuch-config.c +++ b/notmuch-config.c @@ -327,7 +327,7 @@ notmuch_conffile_save (notmuch_conffile_t *config) } /* Try not to overwrite symlinks. */ - filename = canonicalize_file_name (config->filename); + filename = notmuch_canonicalize_file_name (config->filename); if (! filename) { if (errno == ENOENT) { filename = strdup (config->filename); -- Danh _______________________________________________ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-le...@notmuchmail.org