On Sat, Dec 03 2022, Thomas Schneider wrote: > Tomi Ollila <tomi.ollila@iki.fi> writes: > >> On Fri, Dec 02 2022, Thomas Schneider wrote: >> >>> As per strcasestr(3) of glibc and FreeBSD, the header that defines >>> strcasestr() is string.h, not strings.h. This may cause compilation, >>> and thus detection whether an (optimised) version is available, to >>> fail even if the function is available, when implicit declaration and >>> pointer conversion do not match. >>> >>> Signed-off-by: Thomas Schneider <qsx@chaotikum.eu> >>> --- >>> I discovered this when building with Clang: >>> >>> qsx@naboo ~/src/notmuch (git)-[tags/0.32.2] >>> % gcc -o /dev/null compat/have_strcasestr.c; echo $? >>> compat/have_strcasestr.c: In function ‘main’: >>> compat/have_strcasestr.c:10:13: warning: implicit declaration of function ‘strcasestr’; did you mean ‘strcasecmp’? [-Wimplicit-function-declaration] >>> 10 | found = strcasestr (haystack, needle); >>> | ^~~~~~~~~~ >>> | strcasecmp >>> compat/have_strcasestr.c:10:11: warning: assignment to ‘char *’ from ‘int’ makes pointer from integer without a cast [-Wint-conversion] >>> 10 | found = strcasestr (haystack, needle); >>> | ^ >>> 0 >>> qsx@naboo ~/src/notmuch (git)-[tags/0.32.2] >>> % clang -o /dev/null compat/have_strcasestr.c; echo $? >>> compat/have_strcasestr.c:10:13: warning: call to undeclared function 'strcasestr'; ISO C99 and later do not support implicit function declarations [-Wimplicit-function-declaration] >>> found = strcasestr (haystack, needle); >>> ^ >>> compat/have_strcasestr.c:10:11: error: incompatible integer to pointer conversion assigning to 'char *' from 'int' [-Wint-conversion] >>> found = strcasestr (haystack, needle); >>> ^ ~~~~~~~~~~~~~~~~~~~~~~~~~~~~~ >>> 1 warning and 1 error generated. >>> 1 >>> >>> configure then assumes strcasestr is not available when using Clang, so it >>> builds the variant from compat/, which later fails when linking because of >>> conflicting symbols. >>> >>> On a side note, debugging was more complicated that I’m used to, e. g., >>> autoconf’s config.log or similar output. >>> >>> --- >>> compat/have_strcasestr.c | 2 +- >>> 1 file changed, 1 insertion(+), 1 deletion(-) >>> >>> diff --git a/compat/have_strcasestr.c b/compat/have_strcasestr.c >>> index 3cd1838d..d52a62ec 100644 >>> --- a/compat/have_strcasestr.c >>> +++ b/compat/have_strcasestr.c >>> @@ -1,5 +1,5 @@ >>> #define _GNU_SOURCE >>> -#include <strings.h> >>> +#include <string.h> >> >> Would it be better to include both strings.h and string.h, in >> case some (rare) cases it worked only when strings.h were included >> (or would that make the test fail in some cases then...?) > > I don’t think it would cause any issues, but neither would I assume both > headers to be necessary somewhere. Of course I haven’t tested it on Ye > Olde Unix With Its Special Own Taste on a workstation that pales in > comparison to my calculator, then again I assume they either (a) don’t > have strcasestr() anyway, or (b) fail to support notmuch because of some > other issues. > > --Thomas So, string.h is "enough" to declare strcasecmp w/ glibc (-std=gnu*, but not with -std=c99, std=c11 and so on !), any *bsd, probably macOS... ... but do we have *modern* systems where notmuch compiles but strcasecmp() is not declared if strings.h is *not* defined ? I'd write the above lines as #include <strings.h> /* strcasecmp() in POSIX */ #include <string.h> /* strcasecmp() in *BSD */ (or something, if for nothing else to tell the world notmuch developers knows about these things...) Also, it would be nice is commit message were more accurate to tell what is going on... - strings.h is not "incorrect" header file for strcasecmp() - glibc does not declare strcasecmp() in strings.h -- string.h includes strings.h when certain criteria is met (*), (**) Related information: (*) test compilation in linux (fedora 37): $ printf %s\\n '#include <string.h>' \ 'int main(void) { return strcasecmp("a", "b"); }' \ | gcc --std=c11 -xc - <stdin>: In function ‘main’: <stdin>:2:25: warning: implicit declaration of function ‘strcasecmp’; did you mean ‘strncmp’? [-Wimplicit-function-declaration] -- https://pubs.opengroup.org/onlinepubs/009696799/functions/strcasecmp.html includes strings.h -- like linux strcasecmp(3) manual pages. The linux strcasecmp(3) manual page notes (**): The strcasecmp() and strncasecmp() functions first appeared in 4.4BSD, where they were declared in <string.h>. Thus, for reasons of historical compatibility, the glibc <string.h> header file also declares these functions, if the _DEFAULT_SOURCE (or, in glibc 2.19 and earlier, _BSD_SOURCE) feature test macro is defined. >> Tomi Thank you for your efforts to make notmuch better, Tomi _______________________________________________ notmuch mailing list -- notmuch@notmuchmail.org To unsubscribe send an email to notmuch-leave@notmuchmail.org