Re: [PATCH 1/3] configure.ac: check tv_nsec field in struct stat

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Dec 21, 2014 at 1:53 PM, Reuben Hawkins <reubenhwk@xxxxxxxxx> wrote:
> This check will automatically set the correct NO_NSEC setting.

Missing sign-off. See git/Documentation/SubmittingPatches.

> ---
> diff --git a/configure.ac b/configure.ac
> index 6af9647..3cfdd51 100644
> --- a/configure.ac
> +++ b/configure.ac
> @@ -754,6 +754,25 @@ AC_CHECK_TYPES([struct itimerval],
>  [#include <sys/time.h>])
>  GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
>  #
> +# Define HAVE_ST_MTIM=No if you don't have struct stat.st_mtim.tv_nsec.

This comment is misleading. HAVE_ST_MTIM is never actually #defined or
set manually by the user, or used anywhere outside of the conditional
below. Also, the name itself is misleading: HAVE_ST_MTIM_NSEC would be
more appropriate.

> +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec],
> +[HAVE_ST_MTIM=Yes],
> +[HAVE_ST_MTIM=No],

In Autoconf, it's customary to use lowercase values (such as "yes"
rather than "Yes") for these temporary variables. A "no" value is
usually represented by an empty assignment (HAVE_ST_MTIM=). However,
AC_CHECK_MEMBER() assigns the test result automatically to a shell
variable (in this case, named
ac_cv_member_struct_stat_st_mtim_tv_nsec), so there is no need to
invent your own (HAVE_ST_MTIM).

So, you can reduce this to:

   AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])

and later check the value with

    test x$ac_cv_member_struct_stat_st_mtim_tv_nsec = yes

> +[#include <sys/stat.h>])
> +#
> +# Define HAVE_ST_MTIMESPEC=No if you don't have struct stat.st_mtimespec.tv_nsec.

Ditto regarding misleading comment and variable name.

> +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec],
> +[HAVE_ST_MTIMESPEC=Yes],
> +[HAVE_ST_MTIMESPEC=No],
> +[#include <sys/stat.h>])
> +#
> +# Define NO_NSEC if both HAVE_ST_MTIMESPEC and HAVE_ST_MTIM are set to No.
> +if test '(' "$HAVE_ST_MTIM" = "No" ')' -a '(' "$HAVE_ST_MTIMESPEC" = "No" ')' ; then

Use of  'test -a' is unportable. The Autoconf manual has this to say:

    The -a, -o, ‘(’, and ‘)’ operands are not present in all
    implementations, and have been marked obsolete by Posix 2008.

Instead, use:

    test ... && test ....

> +       NO_NSEC=YesPlease
> +       GIT_CONF_SUBST([NO_NSEC])

git-compat-util.h also needs to know if it should use st_ctimespec
rather than st_ctim. By this point, although indirect, you've
determined which of the two is available (if any), so as a bonus, you
can tell the build system about that, as well. Something like this,
perhaps:

    if test x$ac_cv_member_struct_stat_st_mtimspec_tv_nsec = xyes; then
        USE_ST_TIMESPEC=YesPlease
        GIT_CONF_SUBST([USE_ST_TIMESPEC])
    elif test x$ac_cv_member_struct_stat_st_mtime_tv_nsec != xyes; then
        NO_NSEC=YesPlease
        GIT_CONF_SUBST([NO_NSEC])
    fi

> +fi
> +
> +#
>  # Define NO_D_INO_IN_DIRENT if you don't have d_ino in your struct dirent.
>  AC_CHECK_MEMBER(struct dirent.d_ino,
>  [NO_D_INO_IN_DIRENT=],
> --
> 2.2.0.GIT
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]