pg.ddx.io  pgsql-hackers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
macOS 15.4 versus strchrnul()
3+ messages / 2 participants
[nested] [flat]

Commits from this thread:
  2025-04-01 e4440a73c7 915e889680 a39eb9c77f 0de9560ba9 71790aef1a 6da2ba1d8a  Fix detection and handling of strchrnul() for macOS 15.4.
* macOS 15.4 versus strchrnul()
@ 2025-04-01 15:57 Tom Lane <tgl@sss.pgh.pa.us>
  2025-04-01 17:07 ` Re: macOS 15.4 versus strchrnul() Peter Eisentraut <peter@eisentraut.org>
  0 siblings, 1 reply; 3+ messages in thread

From: Tom Lane @ 2025-04-01 15:57 UTC (permalink / raw)
  To: pgsql-hackers@lists.postgresql.org; +Cc: Peter Eisentraut <peter@eisentraut.org>

Last night I updated the machine that hosts sifaka and indri
to spankin' new macOS Sequoia 15.4, and that promptly broke
the build on both animals:

snprintf.c:350:1: error: static declaration of 'strchrnul' follows non-static declaration
  350 | strchrnul(const char *s, int c)
      | ^
/Library/Developer/CommandLineTools/SDKs/MacOSX15.4.sdk/usr/include/_string.h:198:9: note: previous declaration is here
  198 |         strchrnul(const char *__s, int __c);
      |         ^
snprintf.c:414:27: error: 'strchrnul' is only available on macOS 15.4 or newer [-Werror,-Wunguarded-availability-new]
  414 |                         const char *next_pct = strchrnul(format + 1, '%');
      |                                                ^~~~~~~~~
/Library/Developer/CommandLineTools/SDKs/MacOSX15.4.sdk/usr/include/_string.h:198:9: note: 'strchrnul' has been marked as being introduced in macOS 15.4 here, but the deployment target is macOS 15.0.0
  198 |         strchrnul(const char *__s, int __c);
      |         ^
snprintf.c:414:27: note: enclose 'strchrnul' in a __builtin_available check to silence this warning


That is, the function exists now in macOS' libc, and so configure's
does-it-link test for HAVE_STRCHRNUL finds it, but <string.h>
will not let you use it unless you monkey around with

export MACOSX_DEPLOYMENT_TARGET=15.4

or similar.  I don't think we want to require people to do that,
so we need to fix things so that the code works with or without
a deployment target that satisfies <string.h>.  This is pretty
reminiscent of a problem that we faced a couple years ago with
preadv and pwritev, and solved in commit f014b1b9b by depending
on AC_CHECK_DECLS instead of AC_CHECK_FUNCS.  I made a patch
(attached) to solve this similarly.  Interestingly, this actually
makes the one usage in snprintf.c simpler, since we no longer
need to special-case the situation where GNU <string.h> doesn't
agree with the does-it-link test.

However ... testing this here shows that it fixes the autoconf
build as desired, with or without MACOSX_DEPLOYMENT_TARGET.
But the meson version *does not work*: it will set
HAVE_DECL_STRCHRNUL to 1 with or without MACOSX_DEPLOYMENT_TARGET,
and in the "without" case the build then blows up.

I speculate that the meson test for preadv/pwritev has never worked
for macOS either, and we haven't noticed because nobody has tried to
build with meson on a machine with low enough default deployment
target to not have preadv/pwritev.

I do not know nearly enough about meson to fix that test;
can anyone help?

			regards, tom lane

Attachments:

wip-fix-configure-strchrnul-check.patch (text/x-diff, 4.8K) download diff --git a/configure b/configure index 30d949c3c46..3d0e701c745 100755 --- a/configure +++ b/configure @@ -15401,7 +15401,7 @@ fi LIBS_including_readline="$LIBS" LIBS=`echo "$LIBS" | sed -e 's/-ledit//g' -e 's/-lreadline//g'` -for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info getauxval getifaddrs getpeerucred inet_pton kqueue localeconv_l mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strchrnul strsignal syncfs sync_file_range uselocale wcstombs_l +for ac_func in backtrace_symbols copyfile copy_file_range elf_aux_info getauxval getifaddrs getpeerucred inet_pton kqueue localeconv_l mbstowcs_l memset_s posix_fallocate ppoll pthread_is_threaded_np setproctitle setproctitle_fast strsignal syncfs sync_file_range uselocale wcstombs_l do : as_ac_var=`$as_echo "ac_cv_func_$ac_func" | $as_tr_sh` ac_fn_c_check_func "$LINENO" "$ac_func" "$as_ac_var" @@ -15955,6 +15955,18 @@ cat >>confdefs.h <<_ACEOF #define HAVE_DECL_PWRITEV $ac_have_decl _ACEOF +ac_fn_c_check_decl "$LINENO" "strchrnul" "ac_cv_have_decl_strchrnul" "#include <string.h> +" +if test "x$ac_cv_have_decl_strchrnul" = xyes; then : + ac_have_decl=1 +else + ac_have_decl=0 +fi + +cat >>confdefs.h <<_ACEOF +#define HAVE_DECL_STRCHRNUL $ac_have_decl +_ACEOF + # This is probably only present on macOS, but may as well check always ac_fn_c_check_decl "$LINENO" "F_FULLFSYNC" "ac_cv_have_decl_F_FULLFSYNC" "#include <fcntl.h> diff --git a/configure.ac b/configure.ac index 25cdfcf65af..47a287926bc 100644 --- a/configure.ac +++ b/configure.ac @@ -1772,7 +1772,6 @@ AC_CHECK_FUNCS(m4_normalize([ pthread_is_threaded_np setproctitle setproctitle_fast - strchrnul strsignal syncfs sync_file_range @@ -1812,6 +1811,7 @@ AC_CHECK_DECLS([strlcat, strlcpy, strnlen, strsep]) # won't handle deployment target restrictions on macOS AC_CHECK_DECLS([preadv], [], [], [#include <sys/uio.h>]) AC_CHECK_DECLS([pwritev], [], [], [#include <sys/uio.h>]) +AC_CHECK_DECLS([strchrnul], [], [], [#include <string.h>]) # This is probably only present on macOS, but may as well check always AC_CHECK_DECLS(F_FULLFSYNC, [], [], [#include <fcntl.h>]) diff --git a/meson.build b/meson.build index b8da4966297..6932a0f00f7 100644 --- a/meson.build +++ b/meson.build @@ -2577,6 +2577,7 @@ decl_checks = [ decl_checks += [ ['preadv', 'sys/uio.h'], ['pwritev', 'sys/uio.h'], + ['strchrnul', 'string.h'], ] # Check presence of some optional LLVM functions. @@ -2802,7 +2803,6 @@ func_checks = [ ['shm_unlink', {'dependencies': [rt_dep], 'define': false}], ['shmget', {'dependencies': [cygipc_dep], 'define': false}], ['socket', {'dependencies': [socket_dep], 'define': false}], - ['strchrnul'], ['strerror_r', {'dependencies': [thread_dep]}], ['strlcat'], ['strlcpy'], diff --git a/src/include/pg_config.h.in b/src/include/pg_config.h.in index 92f0616c400..2ac61575883 100644 --- a/src/include/pg_config.h.in +++ b/src/include/pg_config.h.in @@ -103,6 +103,10 @@ don't. */ #undef HAVE_DECL_PWRITEV +/* Define to 1 if you have the declaration of `strchrnul', and to 0 if you + don't. */ +#undef HAVE_DECL_STRCHRNUL + /* Define to 1 if you have the declaration of `strlcat', and to 0 if you don't. */ #undef HAVE_DECL_STRLCAT @@ -373,9 +377,6 @@ /* Define to 1 if you have the <stdlib.h> header file. */ #undef HAVE_STDLIB_H -/* Define to 1 if you have the `strchrnul' function. */ -#undef HAVE_STRCHRNUL - /* Define to 1 if you have the `strerror_r' function. */ #undef HAVE_STRERROR_R diff --git a/src/port/snprintf.c b/src/port/snprintf.c index f8f2018ea0c..d0e99fdb072 100644 --- a/src/port/snprintf.c +++ b/src/port/snprintf.c @@ -343,8 +343,15 @@ static void trailing_pad(int padlen, PrintfTarget *target); * * Note: glibc declares this as returning "char *", but that would require * casting away const internally, so we don't follow that detail. + * + * Note: macOS has this too as of Sequoia 15.4, but it's hidden behind + * a deployment-target check that causes compile errors if the deployment + * target isn't high enough. To work around that, use a macro to redefine + * what "strchrnul" means. */ -#ifndef HAVE_STRCHRNUL +#if !HAVE_DECL_STRCHRNUL + +#define strchrnul pg_strchrnul static inline const char * strchrnul(const char *s, int c) @@ -354,19 +361,7 @@ strchrnul(const char *s, int c) return s; } -#else - -/* - * glibc's <string.h> declares strchrnul only if _GNU_SOURCE is defined. - * While we typically use that on glibc platforms, configure will set - * HAVE_STRCHRNUL whether it's used or not. Fill in the missing declaration - * so that this file will compile cleanly with or without _GNU_SOURCE. - */ -#ifndef _GNU_SOURCE -extern char *strchrnul(const char *s, int c); -#endif - -#endif /* HAVE_STRCHRNUL */ +#endif /* !HAVE_DECL_STRCHRNUL */ /*
^ permalink raw reply [nested|flat] 3+ messages in thread

* Re: macOS 15.4 versus strchrnul()
  2025-04-01 15:57 macOS 15.4 versus strchrnul() Tom Lane <tgl@sss.pgh.pa.us>
@ 2025-04-01 17:07 ` Peter Eisentraut <peter@eisentraut.org>
  2025-04-01 19:25   ` Re: macOS 15.4 versus strchrnul() Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 1 reply; 3+ messages in thread

From: Peter Eisentraut @ 2025-04-01 17:07 UTC (permalink / raw)
  To: Tom Lane <tgl@sss.pgh.pa.us>; pgsql-hackers@lists.postgresql.org

On 01.04.25 17:57, Tom Lane wrote:
> That is, the function exists now in macOS' libc, and so configure's
> does-it-link test for HAVE_STRCHRNUL finds it, but <string.h>
> will not let you use it unless you monkey around with
> 
> export MACOSX_DEPLOYMENT_TARGET=15.4
> 
> or similar.  I don't think we want to require people to do that,
> so we need to fix things so that the code works with or without
> a deployment target that satisfies <string.h>.  This is pretty
> reminiscent of a problem that we faced a couple years ago with
> preadv and pwritev, and solved in commit f014b1b9b by depending
> on AC_CHECK_DECLS instead of AC_CHECK_FUNCS.  I made a patch
> (attached) to solve this similarly.  Interestingly, this actually
> makes the one usage in snprintf.c simpler, since we no longer
> need to special-case the situation where GNU <string.h> doesn't
> agree with the does-it-link test.

Agreed, this matches my research.

> However ... testing this here shows that it fixes the autoconf
> build as desired, with or without MACOSX_DEPLOYMENT_TARGET.
> But the meson version *does not work*: it will set
> HAVE_DECL_STRCHRNUL to 1 with or without MACOSX_DEPLOYMENT_TARGET,
> and in the "without" case the build then blows up.
> 
> I speculate that the meson test for preadv/pwritev has never worked
> for macOS either, and we haven't noticed because nobody has tried to
> build with meson on a machine with low enough default deployment
> target to not have preadv/pwritev.

Agreed.  Attached is a patch that implements the test more along the 
lines of how Autoconf does it.  This gives correct results for me for 
strchrnul() in various configurations.

Btw., I see on the buildfarm that strchrnul() is also available on 
FreeBSD, DragonFly BSD, NetBSD, and musl (Alpine Linux).  So perhaps 
some of the comments ought to be rewritten away from that it's a 
glibc-specific extension.

Attachments:

0001-WIP-Fix-AC_CHECK_DECLS-equivalent-in-meson.patch (text/plain, 1.2K) download From e12c5d224a195b893f79c470bc4f21de1585c808 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <peter@eisentraut.org> Date: Tue, 1 Apr 2025 19:00:39 +0200 Subject: [PATCH] WIP: Fix AC_CHECK_DECLS equivalent in meson --- meson.build | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/meson.build b/meson.build index 6932a0f00f7..ba7916d1493 100644 --- a/meson.build +++ b/meson.build @@ -2594,8 +2594,23 @@ foreach c : decl_checks args = c.get(2, {}) varname = 'HAVE_DECL_' + func.underscorify().to_upper() - found = cc.has_header_symbol(header, func, - args: test_c_args, include_directories: postgres_inc, + found = cc.compiles(''' +#include <@0@> + +int main() +{ +#ifndef @1@ + (void) @1@; +#endif + +return 0; +} +'''.format(header, func), + name: 'test whether @0@ is declared'.format(func), + # need to add cflags_warn to get at least + # -Werror=unguarded-availability-new if applicable + args: test_c_args + cflags_warn, + include_directories: postgres_inc, kwargs: args) cdata.set10(varname, found, description: '''Define to 1 if you have the declaration of `@0@', and to 0 if you -- 2.49.0
^ permalink raw reply [nested|flat] 3+ messages in thread

* Re: macOS 15.4 versus strchrnul()
  2025-04-01 15:57 macOS 15.4 versus strchrnul() Tom Lane <tgl@sss.pgh.pa.us>
  2025-04-01 17:07 ` Re: macOS 15.4 versus strchrnul() Peter Eisentraut <peter@eisentraut.org>
@ 2025-04-01 19:25   ` Tom Lane <tgl@sss.pgh.pa.us>
  0 siblings, 0 replies; 3+ messages in thread

From: Tom Lane @ 2025-04-01 19:25 UTC (permalink / raw)
  To: Peter Eisentraut <peter@eisentraut.org>; +Cc: pgsql-hackers@lists.postgresql.org

Peter Eisentraut <peter@eisentraut.org> writes:
> On 01.04.25 17:57, Tom Lane wrote:
>> I speculate that the meson test for preadv/pwritev has never worked
>> for macOS either, and we haven't noticed because nobody has tried to
>> build with meson on a machine with low enough default deployment
>> target to not have preadv/pwritev.

> Agreed.  Attached is a patch that implements the test more along the 
> lines of how Autoconf does it.  This gives correct results for me for 
> strchrnul() in various configurations.

Cool.  Let me try it here, and I'll push if no problems arise.

> Btw., I see on the buildfarm that strchrnul() is also available on 
> FreeBSD, DragonFly BSD, NetBSD, and musl (Alpine Linux).  So perhaps 
> some of the comments ought to be rewritten away from that it's a 
> glibc-specific extension.

OK, will do something about that --- maybe like "originally glibc
specific, but later adopted by other platforms"?

			regards, tom lane





^ permalink  raw  reply  [nested|flat] 3+ messages in thread


end of thread, other threads:[~2025-04-01 19:25 UTC | newest]

Thread overview: 3+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-04-01 15:57 macOS 15.4 versus strchrnul() Tom Lane <tgl@sss.pgh.pa.us>
2025-04-01 17:07 ` Peter Eisentraut <peter@eisentraut.org>
2025-04-01 19:25   ` Tom Lane <tgl@sss.pgh.pa.us>

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox