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 Wed, Jan 7, 2015 at 1:19 PM, Eric Sunshine <sunshine@xxxxxxxxxxxxxx> wrote:
> On Wed, Jan 7, 2015 at 3:23 PM, Reuben Hawkins <reubenhwk@xxxxxxxxx> wrote:
>> This check will automatically set the correct NO_NSEC setting.
>
> This commit message neglects to mention the important point that
> you're also now setting USE_ST_TIMESPEC when detected. You might
> revise the message like this:
>
>     Detect 'tv_nsec' field in 'struct stat' and set Makefile variable
>     NO_NSEC appropriately.
>
>     A side-effect of the above detection is that we also determine
>     whether 'stat.st_mtimespec' is available, so, as a bonus, set the
>     Makefile variable USE_ST_TIMESPEC, as well.
>
> Also, your sign-off is missing (as mentioned in my previous review[1]).
>
> [1]: http://article.gmane.org/gmane.comp.version-control.git/261626
>
>> ---
>>  configure.ac | 12 ++++++++++++
>>  1 file changed, 12 insertions(+)
>>
>> diff --git a/configure.ac b/configure.ac
>> index 6af9647..dcc4bf0 100644
>> --- a/configure.ac
>> +++ b/configure.ac
>> @@ -754,6 +754,18 @@ AC_CHECK_TYPES([struct itimerval],
>>  [#include <sys/time.h>])
>>  GIT_CONF_SUBST([NO_STRUCT_ITIMERVAL])
>>  #
>> +# Define USE_ST_TIMESPEC=YesPlease when stat.st_mtimespec.tv_nsec exist
>
> It would be slightly more accurate to drop the ".tv_nsec" bit from this comment.

The AC_CHECK_MEMBER is checking for st_mtimespec.tv_nsec.  If I drop
tv_nsec from the comment should I also drop it in the check?

I thought it was better to be very explicit because the code using the
check is using that .tv_nsec field...I figured the check may as well
do exactly what the code is doing...


>
> Also: s/exist/exists./
>
>> +# Define NO_NSEC=YesPlease when neither stat.st_mtim.tv_nsec nor stat.st_mtimespec.tv_nsec exist
>
> Perhaps wrap this long comment over two lines.
>
> Also: s/exist/exist./
>
>> +AC_CHECK_MEMBER([struct stat.st_mtimespec.tv_nsec])
>> +AC_CHECK_MEMBER([struct stat.st_mtim.tv_nsec])
>> +if test x$ac_cv_member_struct_stat_st_mtimespec_tv_nsec = xyes ; then
>> +       USE_ST_TIMESPEC=YesPlease
>> +       GIT_CONF_SUBST([USE_ST_TIMESPEC])
>> +elif test x$ac_cv_member_struct_stat_st_mtim_tv_nsec != xyes ; then
>> +       NO_NSEC=YesPlease
>> +       GIT_CONF_SUBST([NO_NSEC])
>> +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.68.g8f72f0c.dirty
--
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]