Linus Torvalds <torvalds@xxxxxxxxxxxxxxxxxxxx> writes:
> On Mon, 17 Mar 2008, Linus Torvalds wrote:
>>
>> IOW, the real "checking" is taking place in "create_file()", so if the
>> unlinking failed (due to a read-only directory or something), that's where
>> we'll do the proper error reporting.
>
> Thinking about this, I'm probably full of sh*t.
>
> My argument is admittedly true in general, but there is one case it is
> *not* true for: if the old entry was a symlink.
>
> IOW, let's imagine that the directory is read-only (or other permission
> issue), and we want to unlink the old symlink, which points somewhere we
> can write to. In that case, the symlink removal is important, because we
> won't necessarily catch the error when we create the file in place later
> (because that will just follow the symlink).
>
> So I retract my statement. We *should* check the result of the unlink.
While I agree we should check the result, I think we are safe against the
un-unlinkable symlink case. If you have a stale symlink at "dir/file"
where you are checking out a new blob, and the directory "dir" the symlink
is in is unwritable, then our callpath would look like this:
checkout_entry()
unlink("dir/file") -- failure silently ignored which is bad
write_entry()
create_file("dir/file")
open("dir/file", O_WRONLY | O_CREAT | O_EXCL)
which would fail, and we get:
error: git-checkout-index: unable to create file a/b (File exists)
from around ll.135 in entry.c::write_entry()
So I'll apply the patch purely as "Root on Solaris safety fix".
--
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
