[PATCH] compat/strcasestr: Include correct header file

[PATCH] compat/strcasestr: Include correct header file

[Thomas Schneider]

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>
 
 int
 main ()
-- 
2.37.4
\r

Re: [PATCH] compat/strcasestr: Include correct header file

[Tomi Ollila]

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...?)


Tomi

>  
>  int
>  main ()
> -- 
> 2.37.4\r

Re: [PATCH] compat/strcasestr: Include correct header file

[Thomas Schneider]

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

> Tomi
>
>>  
>>  int
>>  main ()\r

Re: [PATCH] compat/strcasestr: Include correct header file

[Tomi Ollila]

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\r