Re: [patch v4] manual: add syscall list

["Zack Weinberg" <zack@owlfolio.org>]

On Wed, May 22, 2024, at 3:41 PM, DJ Delorie wrote:
> [v4 cleans up comment text and list prefixes]

I saw something go by earlier about previous reviewers not being
confident in their understanding of the changes to the configure.ac,
so let me jump in and review that part...

> --- a/configure.ac
> +++ b/configure.ac
> @@ -168,6 +168,15 @@ AC_ARG_WITH([timeoutfactor],
>  	    [timeoutfactor=1])
>  AC_DEFINE_UNQUOTED(TIMEOUTFACTOR, $timeoutfactor)
> 
> +man_pages_version=6.8
> +
> +AC_ARG_WITH([man-pages],
> +	    AS_HELP_STRING([--with-man-pages=VERSION],
> +			   [tie manual to a specific man-pages version]),
> +	    [man_pages_version=$withval],
> +	    [])
> +AC_SUBST(man_pages_version)
> +
>  AC_ARG_ENABLE([sanity-checks],
>  	      AS_HELP_STRING([--disable-sanity-checks],
>  			     [really do not use threads (should not be used except in 

This is fine as is.  I might suggest a tiny change: instead of
setting the default value for man_pages_version above the AC_ARG_WITH,
set it in the action-if-not-given:

>  AC_DEFINE_UNQUOTED(TIMEOUTFACTOR, $timeoutfactor)
> 
> +AC_ARG_WITH([man-pages],
> +	    AS_HELP_STRING([--with-man-pages=VERSION],
> +			   [tie manual to a specific man-pages version]),
> +	    [man_pages_version=$withval],
> +	    [man_pages_version=6.8])
> +AC_SUBST(man_pages_version)

Also, I wonder if it makes sense to tie the default here to the
default for --enable-kernel, assuming there is one.

I don’t believe there are any strong reasons to M4-quote or not
M4-quote the argument to AC_SUBST, since shell variable names tend not
to be also M4 macro names, so please just make sure the way you did it
is consistent with AC_SUBSTs elsewhere in the file.


> --- a/config.make.in
> +++ b/config.make.in
> @@ -91,6 +91,7 @@ use-nscd = @use_nscd@
>  build-hardcoded-path-in-tests= @hardcoded_path_in_tests@
>  build-pt-chown = @build_pt_chown@
>  pthread-in-libc = @pthread_in_libc@
> +man-pages-version = @man_pages_version@
> 
>  # Build tools.
>  CC = @CC@

Standard update to expose a new @subst@ as a make variable.

> --- a/configure
> +++ b/configure
...
> @@ -4091,11 +4095,11 @@ if test x$ac_prog_cxx_stdcxx = xno
>  then :
>    { printf "%s\n" "$as_me:${as_lineno-$LINENO}: checking for $CXX 
> option to enable C++11 features" >&5
>  printf %s "checking for $CXX option to enable C++11 features... " >&6; 
> }
> -if test ${ac_cv_prog_cxx_cxx11+y}
> +if test ${ac_cv_prog_cxx_11+y}
>  then :
...

This looks like you didn’t use the same version of Autoconf as the
last person to update configure.ac.  As I understand it we’re trying
to stick to a constant version.  Check with Joseph Myers for what you
should be using.

zw