Re: [PATCH v4] gc: call "prune --expire 2.weeks.ago" by default

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 12, 2008, 23:39 UTC
Message-ID: <7vod9j342h.fsf@gitster.siamese.dyndns.org>
In-Reply-To: <alpine.LSU.1.00.0803130021520.1656@racer.site>

Johannes Schindelin <Johannes.Schindelin@gmx.de> writes:

Show 31 quoted lines

>> Eh, sorry, but why?
>
> The thing is: I want to prevent invalid dates in gc.pruneExpire from going 
> unnoticed, _especially_ since they would default to "now".  IOW if you 
> said something like "one.weak.ago", it would actually have the same effect 
> as "now" and offer _no_ grace period.
>
> But like you said, comparing the difference of two unsigned longs to >= 0 
> might be quite stupid.  Instead, I compare them _directly_.
>
> Since I compare the value to "now" first, and only if it is not, compare 
> the approxidate() of the value to the current time stamp, I can verify 
> that no invalid date was specified.
>
> Unfortunately, this check includes future dates.  Fortunately, they do not 
> make sense at all.
>
> To make my reasoning clear, how about this comment above that if() clause?
>
> 		/*
> 		 * In case of an invalid date, approxidate() returns the
> 		 * same as approxidate("now").  Since the millisecond
> 		 * boundary could have been crossed between the two calls
> 		 * to approxidate(), we compare not only for equality,
> 		 * but also if the former is greater than the latter.
> 		 *
> 		 * Note: this assumes that future dates are invalid, which
> 		 * makes sense, really.
> 		 */
>
> Hmm?

Ah,...

But C language rules haven't changed in such a way that it guarantees B to be evaluated before A when you write "A >= B", have it?

So at least I think you would need something like this if you go that route:

  		if (strcmp(value, "now")) {
                	unsigned long now = approxidate("now");
                	if (approxidate(value) >= now)
				return error("Invalid %s: '%s'", var, value);
			...
		}

Also the resolution of approxidate() is in seconds so millisecond boundary does not matter, but that issue is, eh, secondary ;-).

I have to wonder if approxidate_with_error() function that takes a pointer to receive an error condition may be a better way to solve this cleanly.