depends, NetBSD: Fix `bdb` package compilation with GCC-14 #31613

On NetBSD 10, compilation of the bdb package with GCC-14 fails with the following error:

$ gmake -C depends bdb CC=/usr/pkg/gcc14/bin/gcc
<snip>
./libtool --mode=compile /usr/pkg/gcc14/bin/gcc -c -I. -I../dist/./..   -I/home/hebasto/dev/bitcoin/depends/x86_64-unknown-netbsd10.0/include -D_XOPEN_SOURCE=600  -pipe -std=c11 -O2 -Wno-error=implicit-function-declaration -Wno-error=format-security -Wno-error=implicit-int  ../dist/./../env/env_config.c
libtool: compile:  /usr/pkg/gcc14/bin/gcc -c -I. -I../dist/./.. -I/home/hebasto/dev/bitcoin/depends/x86_64-unknown-netbsd10.0/include -D_XOPEN_SOURCE=600 -pipe -std=c11 -O2 -Wno-error=implicit-function-declaration -Wno-error=format-security -Wno-error=implicit-int ../dist/./../env/env_config.c  -fPIC -DPIC -o env_config.o
../dist/./../env/env_config.c: In function '__config_parse':
../dist/./../env/env_config.c:98:13: warning: implicit declaration of function 'strcasecmp'; did you mean 'strncmp'? [-Wimplicit-function-declaration]
   98 |         if (strcasecmp(s, argv[0]) == 0) {                              \
      |             ^~~~~~~~~~
../dist/./../env/env_config.c:135:9: note: in expansion of macro 'CONFIG_UINT32'
  135 |         CONFIG_UINT32("mutex_set_align", __mutex_set_align);
      |         ^~~~~~~~~~~~~
../dist/./../env/env_config.c: In function '__config_split':
../dist/./../env/env_config.c:537:43: warning: implicit declaration of function 'strsep'; did you mean 'strdup'? [-Wimplicit-function-declaration]
  537 |         for (count = 0, ap = argv; (*ap = strsep(&input, " \t\n")) != NULL;)
      |                                           ^~~~~~
      |                                           strdup
../dist/./../env/env_config.c:537:41: error: assignment to 'char *' from 'int' makes pointer from integer without a cast [-Wint-conversion]
  537 |         for (count = 0, ap = argv; (*ap = strsep(&input, " \t\n")) != NULL;)
      |                                         ^
gmake[1]: *** [Makefile:1664: env_config.o] Error 1
gmake[1]: Leaving directory '/home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/bdb/4.8.30-cac45822ba8/build_unix'
gmake: *** [funcs.mk:302: /home/hebasto/dev/bitcoin/depends/work/build/x86_64-unknown-netbsd10.0/bdb/4.8.30-cac45822ba8/build_unix/.stamp_built] Error 2
gmake: Leaving directory '/home/hebasto/dev/bitcoin/depends'

This PR resolves the compilation issue.

<!--e57a25ab6845829454e8d69fc972939a-->

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

<!--006a51241073e994b41acfe9ec718e94-->

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/31613.

<!--021abf342d371248e50ceaed478a90ca-->

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

<!--174a7506f384e20aa4161008e828411d-->

Conflicts

Reviewers, this pull request conflicts with the following ones:

• #28710 (Remove the legacy wallet and BDB dependency by achow101)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

Why would this be NetBSD-specific? Shouldn't GCC 14 have the same warning/error on every platform?

Wouldn't it be better to add the missing includes? Next to just removing bdb, of course.

Wouldn't it be better to add the missing includes? Next to just removing bdb, of course.

Yeah, it seems that it doesn't know strsep's function signature, so it assumes it returns an int. Not sure why this is a new failure though, I guess it's a new warning with gcc14?

It looks like string.h includes strings.h which defines these functions, but only if _NETBSD_SOURCE is defined. But I'm not sure who's supposed to define that? I suspect that adding it to our cppflags might be the correct thing to do, but who knows what side-effects it would have.

So agreed with @maflcko, let's just add a patch to include strings.h for netbsd.

Also, it seems these were masked by the fact that we use -Wno-error=implicit-function-declaration -Wno-error=implicit-int for depends builds.

This is because exit(0) which is called a bunch during configure (and there are probably a bunch of others too, but exit is the first that causes problems) is implicitly defined because stdlib.h isn't included. Blah.

If this were an important library I'd say we should patch up their configure and remove these warning-off-switches. But since we're going to kill the lib soon anyway, it's not worth the effort.

It looks like string.h includes strings.h which defines these functions, but only if _NETBSD_SOURCE is defined. But I'm not sure who's supposed to define that? I suspect that adding it to our cppflags might be the correct thing to do, but who knows what side-effects it would have.

Defining _NETBSD_SOURCE works, but I agree that it is not an optimal solution, especially when the _XOPEN_SOURCE is already defined.

Also, it seems these were masked by the fact that we use -Wno-error=implicit-function-declaration -Wno-error=implicit-int for depends builds.

This is because exit(0) which is called a bunch during configure (and there are probably a bunch of others too, but exit is the first that causes problems) is implicitly defined because stdlib.h isn't included. Blah.

If this were an important library I'd say we should patch up their configure...

Like here: https://github.com/NetBSD/pkgsrc/blob/pkgsrc-2024Q4/databases/db4/patches/patch-ab.

It looks like string.h includes strings.h which defines these functions, but only if _NETBSD_SOURCE is defined....

So agreed with @maflcko, let's just add a patch to include strings.h for netbsd.

The strsep function declaration is guarded by _NETBSD_SOURCE as well::

#if defined(_NETBSD_SOURCE)
#include <strings.h>		/* for backwards-compatibility */
__BEGIN_DECLS
void	*memmem(const void *, size_t, const void *, size_t);
char	*strcasestr(const char *, const char *);
char	*strchrnul(const char *, int);
size_t	 strlcat(char *, const char *, size_t);
size_t	 strlcpy(char *, const char *, size_t);
char	*strsep(char **, const char *);
char	*stresep(char **, const char *, int);
char	*strnstr(const char *, const char *, size_t);
void	*memrchr(const void *, int, size_t);
void	*explicit_memset(void *, int, size_t);
int	consttime_memequal(const void *, const void *, size_t);
__END_DECLS
#endif

Therefore, including strings.h won't resolve the issue.

But I'm not sure who's supposed to define that?

The https://github.com/NetBSD/pkgsrc repository contains multiple examples demonstrating that this is the responsibility of a package.

In https://github.com/hebasto/bitcoin-core-nightly/pull/31 -D_NETBSD_SOURCE replaces -D_XOPEN_SOURCE=600. Notably, the later was introduced in #26073 without mentioning any tested NetBSD releases. This solution works for both supported NetBSD releases: 10.1 and 9.4.

But since we're going to kill the lib soon anyway, it's not worth the effort.

Yea, this seems like a bit too much time being spent to fix code that is almost dead, that I'd bargain isn't being used on this fairly second-tier (supported) system.