Re: [PATCH 1/1] freshen_file(): use NULL `times' for implicit current-time

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

 



On Tue, Apr 14, 2020 at 04:27:26PM +0200, luciano.rocha@xxxxxxxxxxx wrote:

> Update freshen_file() to use a NULL `times', semantically equivalent to
> the currently setup, with an explicit `actime' and `modtime' set to the
> "current time", but with the advantage that it works with other files
> not owned by the current user.
> 
> Fixes an issue on shared repos with a split index, where eventually a
> user's operation creates a shared index, and another user will later do
> an operation that will try to update its freshness, but will instead
> raise a warning:
>   $ git status
>   warning: could not freshen shared index '.git/sharedindex.bd736fa10e0519593fefdb2aec253534470865b2'

Thanks, this makes sense. And as a bonus, it's less code. ;)

I don't recall having any particular reason not to use NULL in the
original (I may simply not have been aware it was an option), and I
can't find any discussion either way.

One observation:

>  static int freshen_file(const char *fn)
>  {
> -	struct utimbuf t;
> -	t.actime = t.modtime = time(NULL);
> -	return !utime(fn, &t);
> +	return !utime(fn, NULL);
>  }

The old code was setting the time based on the system time from time().
We've occasionally hit cases where the filesystem time and the system
time do not match exactly (this might be true on an NFS mount, for
example).

It's not clear to me whether utime(NULL) would be using the system or
filesystem time in such a case. If the former, then there's no change in
behavior. If the latter, I'd argue that it's probably an _improvement_,
since we're simulating the case that we wrote a new file with a new
mtime.

-Peff



[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]

  Powered by Linux