Thread (7 messages) flat view 7 messages, 3 authors, 2016-06-15

Re: [PATCH 2/2] read-cache.c: fix timestamp comparison

From: Linus Torvalds <torvalds@linux-foundation.org>
Date: 2016-06-15 22:44:07

Possibly related (same subject, not in this thread)


On Mon, 21 Jan 2008, Johannes Schindelin wrote:
Not that I understand what is _really_ going on, but shouldn't the comment 
be actually moved, not be deleted?
Well, the thing is, it's not a special case any more, and you can now see 
the code, and say "that's obviously correct".

The whole point of that function is to compare a index entry with the stat 
information, and it does that by validating it in special ways. It used to 
be that the "ce_mode = 0" was a special case. Now it isn't.

A deleted entry can obviously never match an entry that is still on disk 
(regardless of *any* other issues). So now the

	if (ce->ce_flags & CE_REMOVE)
		return MODE_CHANGED | DATA_CHANGED | TYPE_CHANGED;

statement is in no way a special case - rather the reverse, it's a lot 
more obvious than testing for inode ownership changes etc.

So I removed the comment, because it doesn't make sense any more.

Of course, I could have - instead of deleting it - changed it from 
"Special case: .." to something like "A deleted index entry doesn't match 
any on-disk file", but does that actually add any information to the above 
two lines? Sure, we can comment things, but shouldn't we comment the ones 
that are subtle or odd, rather than the obvious ones?

This was why the CE_REMOVE bit was done in the first place: to remove the 
rather special case of ce_mode being zero meaning something special.

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