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

4 messages, 4 authors, 2016-06-15 · open the first message on its own page

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

From: Junio C Hamano <hidden>
Date: 2016-06-15 22:44:22

Johannes Schindelin [off-list ref] writes:
quoted
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.

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

From: Johannes Schindelin <hidden>
Date: 2016-06-15 22:44:22

Hi,

On Wed, 12 Mar 2008, Junio C Hamano wrote:
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.
Right.  But that will have to wait for at least tomorrow, if it waits for 
me.

Ciao,
Dscho

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

From: Wincent Colaiuta <hidden>
Date: 2016-06-15 22:44:22

El 13/3/2008, a las 0:39, Junio C Hamano escribió:
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);
			...
		}

Are you sure that that alternative provides any guarantees about  
evaluation order either? (I'm not a compiler expert, nor am I  
consulting a copy of the standard; but I don't think it does.)

In order to enforce evaluation in the required order I think it might  
have to be something like this:

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

ie. the && operator guarantees left to right evaluation order, and  
since approxidate always returns a non-zero ulong the second  
expression (after the &&) will always be evaluated.

Cheers,
Wincent

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

From: Johannes Sixt <hidden>
Date: 2016-06-15 22:44:22

Wincent Colaiuta schrieb:
El 13/3/2008, a las 0:39, Junio C Hamano escribió:
quoted
         if (strcmp(value, "now")) {
                   unsigned long now = approxidate("now");
                   if (approxidate(value) >= now)
                return error("Invalid %s: '%s'", var, value);
            ...
        }

Are you sure that that alternative provides any guarantees about
evaluation order either? (I'm not a compiler expert, nor am I consulting
a copy of the standard; but I don't think it does.)
There is a sequence point at the semicolon. This means that all observable
side effects of approxidate("now") are visible when approxidate(value) is
evaluated (and no observable side effect of the latter is visible when the
former is evaluated), which doesn't leave much choice for the compiler.
So, yes, this does guarantee the intended evaluation order.

-- Hannes
Keyboard shortcuts
hback out one level
jnext message in thread
kprevious message in thread
ldrill in
Escclose help / fold thread tree
?toggle this help