Re: [PATCH] compat: expose canonicalize_file_name to C++

Subject: Re: [PATCH] compat: expose canonicalize_file_name to C++

Date: Sat, 24 Apr 2021 07:29:46 +0700

To: David Bremner

Cc: Tomi Ollila, notmuch@notmuchmail.org

From: Đoàn Trần Công Danh


On 2021-04-23 14:31:43-0300, David Bremner <david@tethera.net> wrote:
> Đoàn Trần Công Danh <congdanhqx@gmail.com> writes:
> 
> >
> > Yes, inline function are always better than macro.
> > I feel embarassed that I couldn't think about that earlier.
> >
> > Here is a revised patch:
> 
> This version still has some issues on a glibc system.
> 
> ,----
> | In file included from notmuch-client.h:31,
> |                  from debugger.c:21:
> | ./compat/compat.h: In function ‘notmuch_canonicalize_file_name’:
> | ./compat/compat.h:44:9: warning: implicit declaration of function ‘canonicalize_file_name’; did you mean ‘notmuch_canonicalize_file_name’? [-Wimplicit-function-declaration]
> |    44 |  return canonicalize_file_name (path);
> |       |         ^~~~~~~~~~~~~~~~~~~~~~
> |       |         notmuch_canonicalize_file_name
> | ./compat/compat.h:44:9: warning: returning ‘int’ from a function with return type ‘char *’ makes pointer from integer without a cast [-Wint-conversion]
> |    44 |  return canonicalize_file_name (path);
> |       |         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~
> `----
> 
> I guess compat.h needs to define _GNU_SOURCE and include stdlib.h?
> 
> ,----
> | /usr/include/stdlib.h:790:14: error: conflicting types for ‘canonicalize_file_name’
> |   790 | extern char *canonicalize_file_name (const char *__name)
> |       |              ^~~~~~~~~~~~~~~~~~~~~~
> | In file included from notmuch-client.h:31,
> |                  from debugger.c:21:
> | ./compat/compat.h:44:9: note: previous implicit declaration of ‘canonicalize_file_name’ was here
> |    44 |  return canonicalize_file_name (path);
> |       |         ^~~~~~~~~~~~~~~~~~~~~~
> | make: *** [Makefile.local:204: debugger.o] Error 1
> `----
> 
> I guess that's the same issue?
> 
> I was thinking if we are going to have provide
> notmuch_canonicalize_file_name, it might make more sense to start e.g.
> util/path-util.{c,h}, and put the relevant inline (or not) function
> there.  I guess either the current approach (with appropriate fixes), or
> putting things in util is fine.

I've tried to put

	#define _GNU_SOURCE
	#include <stdlib.h>

in compat.h for HAVE_CANONICALIZE_FILE_NAME

However, other source files include stdlib.h w/o _GNU_SOURCE before
compat.h, thus declines to include stdlib.h again.

If we prefer to add above code into compat.h, I think it'd be an
invasive change (move all #include "compat.h") as first inclusion in
all files, for a long run, it sounds like a better move. I don't think
it's wise for me to go with that route, though.

Starting util/path-util.{c,h} sounds like a nice alternate to me.

-- 
Danh
_______________________________________________
notmuch mailing list -- notmuch@notmuchmail.org
To unsubscribe send an email to notmuch-leave@notmuchmail.org

Thread: