[PATCH] write_sha1_buffer

[PATCH] write_sha1_buffer

[Morten Welinder]

This write will failing sooner or later when someone's disk fills up. 
That'll leave someone with
a truncated file.

Signed-off-by: Morten Welinder <mwelinder@gmail.com>


--- read-cache.c
+++ read-cache.c        2005-04-15 20:32:52.111187168 -0400
@@ -276,9 +276,13 @@
                                        " This is bad, bad, BAD!\a\n");
                return 0;
        }
-       write(fd, buf, size);
-       close(fd);
-       return 0;
+
+       if (write(fd, buf, size) != size) {
+               close(fd);
+               return error("Failed to write file %s\n", filename);
+       }
+
+       return close(fd);
 }

Re: [PATCH] write_sha1_buffer

[Linus Torvalds]

On Fri, 15 Apr 2005, Morten Welinder wrote:
>
> This write will failing sooner or later when someone's disk fills up. 
> That'll leave someone with a truncated file.

Yes. On the other hand, we could try to do this even better, ie make the 
classic write loop that handles EAGAIN.

No POSIX filesystem is supposed to return EAGAIN, but there are tons of 
"POSIX enough" filesystems. Notably NFS when mounted with "intr" (which 
some people think is wrong, but it tends to be better than the 
alternatives if your network is flaky enough).

But yes, even just a "write failed" is good enough, except you should also 
make sure that you remove the corrupt file. Sure, fsck will catch it, but 
if you don't do an fsck, somebody else might decide not to write the file 
out simply because "it's already there".

(This is also why we should write to a temp-file and then do an atomic
"rename()").

		Linus

Re: [PATCH] write_sha1_buffer

[Linus Torvalds]

On Fri, 15 Apr 2005, Linus Torvalds wrote:
> 
> (This is also why we should write to a temp-file and then do an atomic
> "rename()").

Btw, before anybody asks: I do _not_ think that we should do fsync() etc. 
We don't actually destroy any old state when we write a new object, so 
even if the machine does go down, we really should just do an fsck and 
then re-try the operation. 

Anal people (or people with machines that crash) can add it later if they 
really want.

		Linus