* notmuch-0.16: realpath() compatibility issue; clang visibility problem
@ 2014-01-03 21:47 Thomas Klausner
2014-01-04 12:35 ` Jani Nikula
` (3 more replies)
0 siblings, 4 replies; 20+ messages in thread
From: Thomas Klausner @ 2014-01-03 21:47 UTC (permalink / raw)
To: notmuch
[-- Attachment #1: Type: text/plain, Size: 2337 bytes --]
Hi!
I'm currently starting to try out notmuch-0.16 on NetBSD. It went off
to a rocky start, since it segfaulted in the initial config setup.
Debugging it I found that notmuch uses a glibc extension to realpath,
allowing NULL as second argument.
I've converted it to use a prepared buffer instead; attached is a
possible patch that makes notmuch complete its setup phase for me, and
adds inclusion of the header files suggested by the realpath man page
on NetBSD. Please address this issue in some way in the next release.
Additionally, when compiling with clang, there are issues with the
visibility. The symptoms are:
In file included from lib/database.cc:21:
In file included from ./lib/database-private.h:33:
./lib/notmuch-private.h:479:8: error: visibility does not match previous declaration
array subscriptstruct visible _notmuch_string_list {
^
./lib/notmuch-private.h:67:33: note: expanded from macro 'visible'
# define visible __attribute__((visibility("default")))
^
./lib/notmuch-private.h:52:13: note: previous attribute is here
#pragma GCC visibility push(hidden)
^
In file included from lib/parse-time-vrp.cc:23:
In file included from ./lib/database-private.h:33:
./lib/notmuch-private.h:479:8: error: visibility does not match previous declaration
struct visible _notmuch_string_list {
^
./lib/notmuch-private.h:67:33: note: expanded from macro 'visible'
# define visible __attribute__((visibility("default")))
^
./lib/notmuch-private.h:52:13: note: previous attribute is here
#pragma GCC visibility push(hidden)
^
1 warning generated.
In file included from lib/directory.cc:21:
./lib/notmuch-private.h:479:8: error: visibility does not match previous declaration
struct visible _notmuch_string_list {
^
./lib/notmuch-private.h:67:33: note: expanded from macro 'visible'
# define visible __attribute__((visibility("default")))
^
./lib/notmuch-private.h:52:13: note: previous attribute is here
#pragma GCC visibility push(hidden)
^
and so on. I guess it is because the visibility differs between c and
c++. I've disabled visibility locally, see second attached patch, but
of course that's not a solution, just a workaround. Suggestions
welcome.
Thanks,
Thomas
[-- Attachment #2: patch-notmuch-config.c --]
[-- Type: text/plain, Size: 1497 bytes --]
$NetBSD$
NULL as second argument for realpath() is only supported in glibc.
Use more portable code.
--- notmuch-config.c.orig 2013-08-03 11:29:40.000000000 +0000
+++ notmuch-config.c
@@ -23,6 +23,8 @@
#include <pwd.h>
#include <netdb.h>
#include <assert.h>
+#include <sys/param.h>
+#include <stdlib.h>
static const char toplevel_config_comment[] =
" .notmuch-config - Configuration file for the notmuch mail system\n"
@@ -444,7 +446,7 @@ int
notmuch_config_save (notmuch_config_t *config)
{
size_t length;
- char *data, *filename;
+ char *data, filename[MAXPATHLEN];
GError *error = NULL;
data = g_key_file_to_data (config->key_file, &length, NULL);
@@ -454,15 +456,9 @@ notmuch_config_save (notmuch_config_t *c
}
/* Try not to overwrite symlinks. */
- filename = realpath (config->filename, NULL);
- if (! filename) {
+ if (! realpath (config->filename, filename)) {
if (errno == ENOENT) {
- filename = strdup (config->filename);
- if (! filename) {
- fprintf (stderr, "Out of memory.\n");
- g_free (data);
- return 1;
- }
+ strcpy(filename, config->filename);
} else {
fprintf (stderr, "Error canonicalizing %s: %s\n", config->filename,
strerror (errno));
@@ -480,12 +476,10 @@ notmuch_config_save (notmuch_config_t *c
filename, error->message);
}
g_error_free (error);
- free (filename);
g_free (data);
return 1;
}
- free (filename);
g_free (data);
return 0;
}
[-- Attachment #3: patch-lib_notmuch-private.h --]
[-- Type: text/plain, Size: 346 bytes --]
$NetBSD$
Doesn't compile with clang.
--- lib/notmuch-private.h.orig 2013-08-03 11:29:40.000000000 +0000
+++ lib/notmuch-private.h
@@ -64,7 +64,7 @@ NOTMUCH_BEGIN_DECLS
#define unused(x) x __attribute__ ((unused))
#ifdef __cplusplus
-# define visible __attribute__((visibility("default")))
+# define visible
#else
# define visible
#endif
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-03 21:47 notmuch-0.16: realpath() compatibility issue; clang visibility problem Thomas Klausner
@ 2014-01-04 12:35 ` Jani Nikula
2014-01-04 12:46 ` Jani Nikula
2014-01-04 12:52 ` Thomas Klausner
2014-01-04 13:18 ` David Bremner
` (2 subsequent siblings)
3 siblings, 2 replies; 20+ messages in thread
From: Jani Nikula @ 2014-01-04 12:35 UTC (permalink / raw)
To: Thomas Klausner; +Cc: Notmuch Mail
[-- Attachment #1: Type: text/plain, Size: 2726 bytes --]
For the visibility issue please upgrade Notmuch.
BR,
Jani.
On Jan 4, 2014 2:26 PM, "Thomas Klausner" <tk@giga.or.at> wrote:
>
> Hi!
>
> I'm currently starting to try out notmuch-0.16 on NetBSD. It went off
> to a rocky start, since it segfaulted in the initial config setup.
>
> Debugging it I found that notmuch uses a glibc extension to realpath,
> allowing NULL as second argument.
>
> I've converted it to use a prepared buffer instead; attached is a
> possible patch that makes notmuch complete its setup phase for me, and
> adds inclusion of the header files suggested by the realpath man page
> on NetBSD. Please address this issue in some way in the next release.
>
> Additionally, when compiling with clang, there are issues with the
> visibility. The symptoms are:
>
> In file included from lib/database.cc:21:
> In file included from ./lib/database-private.h:33:
> ./lib/notmuch-private.h:479:8: error: visibility does not match previous
declaration
> array subscriptstruct visible _notmuch_string_list {
> ^
> ./lib/notmuch-private.h:67:33: note: expanded from macro 'visible'
> # define visible __attribute__((visibility("default")))
> ^
> ./lib/notmuch-private.h:52:13: note: previous attribute is here
> #pragma GCC visibility push(hidden)
> ^
>
> In file included from lib/parse-time-vrp.cc:23:
> In file included from ./lib/database-private.h:33:
> ./lib/notmuch-private.h:479:8: error: visibility does not match previous
declaration
> struct visible _notmuch_string_list {
> ^
> ./lib/notmuch-private.h:67:33: note: expanded from macro 'visible'
> # define visible __attribute__((visibility("default")))
> ^
> ./lib/notmuch-private.h:52:13: note: previous attribute is here
> #pragma GCC visibility push(hidden)
> ^
> 1 warning generated.
> In file included from lib/directory.cc:21:
> ./lib/notmuch-private.h:479:8: error: visibility does not match previous
declaration
> struct visible _notmuch_string_list {
> ^
> ./lib/notmuch-private.h:67:33: note: expanded from macro 'visible'
> # define visible __attribute__((visibility("default")))
> ^
> ./lib/notmuch-private.h:52:13: note: previous attribute is here
> #pragma GCC visibility push(hidden)
> ^
>
> and so on. I guess it is because the visibility differs between c and
> c++. I've disabled visibility locally, see second attached patch, but
> of course that's not a solution, just a workaround. Suggestions
> welcome.
>
> Thanks,
> Thomas
>
> _______________________________________________
> notmuch mailing list
> notmuch@notmuchmail.org
> http://notmuchmail.org/mailman/listinfo/notmuch
>
[-- Attachment #2: Type: text/html, Size: 3617 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-04 12:35 ` Jani Nikula
@ 2014-01-04 12:46 ` Jani Nikula
2014-01-04 12:53 ` Thomas Klausner
2014-01-04 12:52 ` Thomas Klausner
1 sibling, 1 reply; 20+ messages in thread
From: Jani Nikula @ 2014-01-04 12:46 UTC (permalink / raw)
To: Thomas Klausner; +Cc: Notmuch Mail
[-- Attachment #1: Type: text/plain, Size: 3089 bytes --]
I guess we should look at realpath() compatibility, but in fairness passing
NULL for the second parameter is according to POSIX.1-2008, not glibc
extension.
On Jan 4, 2014 2:35 PM, "Jani Nikula" <jani@nikula.org> wrote:
>
> For the visibility issue please upgrade Notmuch.
>
> BR,
> Jani.
>
> On Jan 4, 2014 2:26 PM, "Thomas Klausner" <tk@giga.or.at> wrote:
> >
> > Hi!
> >
> > I'm currently starting to try out notmuch-0.16 on NetBSD. It went off
> > to a rocky start, since it segfaulted in the initial config setup.
> >
> > Debugging it I found that notmuch uses a glibc extension to realpath,
> > allowing NULL as second argument.
> >
> > I've converted it to use a prepared buffer instead; attached is a
> > possible patch that makes notmuch complete its setup phase for me, and
> > adds inclusion of the header files suggested by the realpath man page
> > on NetBSD. Please address this issue in some way in the next release.
> >
> > Additionally, when compiling with clang, there are issues with the
> > visibility. The symptoms are:
> >
> > In file included from lib/database.cc:21:
> > In file included from ./lib/database-private.h:33:
> > ./lib/notmuch-private.h:479:8: error: visibility does not match
previous declaration
> > array subscriptstruct visible _notmuch_string_list {
> > ^
> > ./lib/notmuch-private.h:67:33: note: expanded from macro 'visible'
> > # define visible __attribute__((visibility("default")))
> > ^
> > ./lib/notmuch-private.h:52:13: note: previous attribute is here
> > #pragma GCC visibility push(hidden)
> > ^
> >
> > In file included from lib/parse-time-vrp.cc:23:
> > In file included from ./lib/database-private.h:33:
> > ./lib/notmuch-private.h:479:8: error: visibility does not match
previous declaration
> > struct visible _notmuch_string_list {
> > ^
> > ./lib/notmuch-private.h:67:33: note: expanded from macro 'visible'
> > # define visible __attribute__((visibility("default")))
> > ^
> > ./lib/notmuch-private.h:52:13: note: previous attribute is here
> > #pragma GCC visibility push(hidden)
> > ^
> > 1 warning generated.
> > In file included from lib/directory.cc:21:
> > ./lib/notmuch-private.h:479:8: error: visibility does not match
previous declaration
> > struct visible _notmuch_string_list {
> > ^
> > ./lib/notmuch-private.h:67:33: note: expanded from macro 'visible'
> > # define visible __attribute__((visibility("default")))
> > ^
> > ./lib/notmuch-private.h:52:13: note: previous attribute is here
> > #pragma GCC visibility push(hidden)
> > ^
> >
> > and so on. I guess it is because the visibility differs between c and
> > c++. I've disabled visibility locally, see second attached patch, but
> > of course that's not a solution, just a workaround. Suggestions
> > welcome.
> >
> > Thanks,
> > Thomas
> >
> > _______________________________________________
> > notmuch mailing list
> > notmuch@notmuchmail.org
> > http://notmuchmail.org/mailman/listinfo/notmuch
> >
[-- Attachment #2: Type: text/html, Size: 4266 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-04 12:46 ` Jani Nikula
@ 2014-01-04 12:53 ` Thomas Klausner
2014-01-04 13:06 ` Tomi Ollila
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Klausner @ 2014-01-04 12:53 UTC (permalink / raw)
To: Jani Nikula; +Cc: Notmuch Mail
On Sat, Jan 04, 2014 at 02:46:37PM +0200, Jani Nikula wrote:
> I guess we should look at realpath() compatibility, but in fairness passing
> NULL for the second parameter is according to POSIX.1-2008, not glibc
> extension.
Ah, interesting.
I can file a bug report with NetBSD about that, but in the meantime,
it causes a coredump. :|
Thanks,
Thomas
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-04 12:53 ` Thomas Klausner
@ 2014-01-04 13:06 ` Tomi Ollila
2014-01-04 13:28 ` Thomas Klausner
0 siblings, 1 reply; 20+ messages in thread
From: Tomi Ollila @ 2014-01-04 13:06 UTC (permalink / raw)
To: Thomas Klausner; +Cc: Notmuch Mail
On Sat, Jan 04 2014, Thomas Klausner <tk@giga.or.at> wrote:
> On Sat, Jan 04, 2014 at 02:46:37PM +0200, Jani Nikula wrote:
>> I guess we should look at realpath() compatibility, but in fairness passing
>> NULL for the second parameter is according to POSIX.1-2008, not glibc
>> extension.
>
> Ah, interesting.
>
> I can file a bug report with NetBSD about that, but in the meantime,
> it causes a coredump. :|
The linux namual page (*) has good explanation of this in the BUGS section
(*) http://man7.org/linux/man-pages/man3/realpath.3.html
After reading that I think fixing that is not as simple as your previous
patch does it -- so probably you just have to keep patching your system
until NetBSD library realpath() is POSIX.1-2008 -compatible.
Note that having users' own patches in their systems is not uncommon
at all :D
>
> Thanks,
> Thomas
Tomi
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-04 13:06 ` Tomi Ollila
@ 2014-01-04 13:28 ` Thomas Klausner
0 siblings, 0 replies; 20+ messages in thread
From: Thomas Klausner @ 2014-01-04 13:28 UTC (permalink / raw)
To: Tomi Ollila; +Cc: Notmuch Mail
On Sat, Jan 04, 2014 at 03:06:38PM +0200, Tomi Ollila wrote:
> The linux namual page (*) has good explanation of this in the BUGS section
>
> (*) http://man7.org/linux/man-pages/man3/realpath.3.html
>
> After reading that I think fixing that is not as simple as your previous
> patch does it -- so probably you just have to keep patching your system
> until NetBSD library realpath() is POSIX.1-2008 -compatible.
Ok, so it's not that easy in general. On the other hand, I wonder how
many systems support POSIX.1-2008 realpath() :)
> Note that having users' own patches in their systems is not uncommon
> at all :D
I've filed a bug report in the meantime:
http://gnats.netbsd.org/48497
Thanks for the comments,
Thomas
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-04 12:35 ` Jani Nikula
2014-01-04 12:46 ` Jani Nikula
@ 2014-01-04 12:52 ` Thomas Klausner
1 sibling, 0 replies; 20+ messages in thread
From: Thomas Klausner @ 2014-01-04 12:52 UTC (permalink / raw)
To: Jani Nikula; +Cc: Notmuch Mail
On Sat, Jan 04, 2014 at 02:35:54PM +0200, Jani Nikula wrote:
> For the visibility issue please upgrade Notmuch.
Thanks. It seems 0.17 came out a short time after I downloaded notmuch :)
I don't need the visibility patch with that version any longer.
Thomas
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-03 21:47 notmuch-0.16: realpath() compatibility issue; clang visibility problem Thomas Klausner
2014-01-04 12:35 ` Jani Nikula
@ 2014-01-04 13:18 ` David Bremner
2014-01-04 22:37 ` Thomas Klausner
2014-04-08 11:26 ` David Bremner
2017-03-12 16:00 ` notmuch-0.16: realpath() compatibility issue; clang visibility problem David Bremner
3 siblings, 1 reply; 20+ messages in thread
From: David Bremner @ 2014-01-04 13:18 UTC (permalink / raw)
To: Thomas Klausner, notmuch
Thomas Klausner <tk@giga.or.at> writes:
> ^
> ./lib/notmuch-private.h:52:13: note: previous attribute is here
> #pragma GCC visibility push(hidden)
> ^
The clang related issues might be fixed in 0.17; can you try that (or
git master)?
> size_t length;
> - char *data, *filename;
> + char *data, filename[MAXPATHLEN];
> GError *error = NULL;
I'm not sure what the right answer is here. MATHPATHLEN (and PATH_MAX)
are not necessarily defined; in particular this would break
compilation on GNU Hurd. Perhaps we should ship a compatibility
implementation of a POSIX.1-2008 compatible [1] realpath. Or maybe
realpath can be avoided completely here.
> + strcpy(filename, config->filename);
Any reason not to use strncpy here?
Of course bug reports and fixes in any form are always welcome, but even
more appreciated if they roughly follow [2]; mainly patches from git
with sensible commit messages, and some minor coding style issues.
cheers,
d
[1]: http://pubs.opengroup.org/onlinepubs/9699919799/
[2]: http://notmuchmail.org/contributing/
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-04 13:18 ` David Bremner
@ 2014-01-04 22:37 ` Thomas Klausner
2014-01-04 22:53 ` Jani Nikula
0 siblings, 1 reply; 20+ messages in thread
From: Thomas Klausner @ 2014-01-04 22:37 UTC (permalink / raw)
To: David Bremner; +Cc: notmuch
On Sat, Jan 04, 2014 at 09:18:15AM -0400, David Bremner wrote:
> Thomas Klausner <tk@giga.or.at> writes:
>
> > ^
> > ./lib/notmuch-private.h:52:13: note: previous attribute is here
> > #pragma GCC visibility push(hidden)
> > ^
>
> The clang related issues might be fixed in 0.17; can you try that (or
> git master)?
Yes, 0.17 fixed that problem.
> > size_t length;
> > - char *data, *filename;
> > + char *data, filename[MAXPATHLEN];
> > GError *error = NULL;
>
> I'm not sure what the right answer is here. MATHPATHLEN (and PATH_MAX)
> are not necessarily defined; in particular this would break
> compilation on GNU Hurd. Perhaps we should ship a compatibility
> implementation of a POSIX.1-2008 compatible [1] realpath. Or maybe
> realpath can be avoided completely here.
A compatibility implementation for POSIX.1-2008-realpath would be
great, as would be avoiding the call. Why is it necessary to resolve
$HOME here?
> > + strcpy(filename, config->filename);
>
> Any reason not to use strncpy here?
You're right, that'd be better here.
> Of course bug reports and fixes in any form are always welcome, but even
> more appreciated if they roughly follow [2]; mainly patches from git
> with sensible commit messages, and some minor coding style issues.
Thanks for the comments,
Thomas
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-04 22:37 ` Thomas Klausner
@ 2014-01-04 22:53 ` Jani Nikula
0 siblings, 0 replies; 20+ messages in thread
From: Jani Nikula @ 2014-01-04 22:53 UTC (permalink / raw)
To: Thomas Klausner; +Cc: Notmuch Mail
[-- Attachment #1: Type: text/plain, Size: 664 bytes --]
On Jan 5, 2014 12:38 AM, "Thomas Klausner" <tk@giga.or.at> wrote:
>
> On Sat, Jan 04, 2014 at 09:18:15AM -0400, David Bremner wrote:
> > I'm not sure what the right answer is here. MATHPATHLEN (and PATH_MAX)
> > are not necessarily defined; in particular this would break
> > compilation on GNU Hurd. Perhaps we should ship a compatibility
> > implementation of a POSIX.1-2008 compatible [1] realpath. Or maybe
> > realpath can be avoided completely here.
>
> A compatibility implementation for POSIX.1-2008-realpath would be
> great, as would be avoiding the call. Why is it necessary to resolve
> $HOME here?
realpath is used to follow, not overwrite symlinks.
[-- Attachment #2: Type: text/html, Size: 853 bytes --]
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-03 21:47 notmuch-0.16: realpath() compatibility issue; clang visibility problem Thomas Klausner
2014-01-04 12:35 ` Jani Nikula
2014-01-04 13:18 ` David Bremner
@ 2014-04-08 11:26 ` David Bremner
[not found] ` <20140408123312.GZ5053@danbala.tuwien.ac.at>
2017-03-12 16:00 ` notmuch-0.16: realpath() compatibility issue; clang visibility problem David Bremner
3 siblings, 1 reply; 20+ messages in thread
From: David Bremner @ 2014-04-08 11:26 UTC (permalink / raw)
To: Thomas Klausner, notmuch
Thomas Klausner <tk@giga.or.at> writes:
>
> Debugging it I found that notmuch uses a glibc extension to realpath,
> allowing NULL as second argument.
>
This should be fixed in commit af5c3af ; I'd appreciate if you can test
it.
d
^ permalink raw reply [flat|nested] 20+ messages in thread
* Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem
2014-01-03 21:47 notmuch-0.16: realpath() compatibility issue; clang visibility problem Thomas Klausner
` (2 preceding siblings ...)
2014-04-08 11:26 ` David Bremner
@ 2017-03-12 16:00 ` David Bremner
3 siblings, 0 replies; 20+ messages in thread
From: David Bremner @ 2017-03-12 16:00 UTC (permalink / raw)
To: Thomas Klausner, notmuch
Thomas Klausner <tk@giga.or.at> writes:
> Hi!
>
> I'm currently starting to try out notmuch-0.16 on NetBSD. It went off
> to a rocky start, since it segfaulted in the initial config setup.
>
> Debugging it I found that notmuch uses a glibc extension to realpath,
> allowing NULL as second argument.
This exact bug was fixed long ago, tagging fixed in nmbug.
^ permalink raw reply [flat|nested] 20+ messages in thread
end of thread, other threads:[~2017-03-12 20:58 UTC | newest]
Thread overview: 20+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-01-03 21:47 notmuch-0.16: realpath() compatibility issue; clang visibility problem Thomas Klausner
2014-01-04 12:35 ` Jani Nikula
2014-01-04 12:46 ` Jani Nikula
2014-01-04 12:53 ` Thomas Klausner
2014-01-04 13:06 ` Tomi Ollila
2014-01-04 13:28 ` Thomas Klausner
2014-01-04 12:52 ` Thomas Klausner
2014-01-04 13:18 ` David Bremner
2014-01-04 22:37 ` Thomas Klausner
2014-01-04 22:53 ` Jani Nikula
2014-04-08 11:26 ` David Bremner
[not found] ` <20140408123312.GZ5053@danbala.tuwien.ac.at>
2014-06-26 12:02 ` David Bremner
2014-06-26 12:52 ` David Bremner
2014-06-26 13:08 ` notmuch-0.18 issues [was Re: notmuch-0.16: realpath() compatibility issue; clang visibility problem] Thomas Klausner
2014-06-26 13:16 ` Tomi Ollila
2014-06-26 15:00 ` David Bremner
2017-03-12 16:21 ` David Bremner
2017-03-12 17:24 ` Tomi Ollila
2017-03-12 20:52 ` Thomas Klausner
2017-03-12 16:00 ` notmuch-0.16: realpath() compatibility issue; clang visibility problem David Bremner
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).