threads / patch / 40

patchwrite_sha1_buffer

Subject: [PATCH] write_sha1_buffer

## tl;dr

3 messages between Apr 16, 2005 and Apr 16, 2005. Diffs are folded; open one to read it.

replies: 2people: 2as markdown or json

Morten Welinder· Apr 16, 2005, 00:50 UTC · lore

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

Show changes to diff +7 −3

@@ -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);
 }

Linus Torvalds· Apr 16, 2005, 02:55 UTC · re: Morten Welinder · lore

Re: [PATCH] write_sha1_buffer

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

Linus Torvalds· Apr 16, 2005, 02:59 UTC · re: Linus Torvalds · lore

Re: [PATCH] write_sha1_buffer

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