Git development
 help / color / mirror / Atom feed
From: Andy Whitcroft <apw@shadowen.org>
To: Linus Torvalds <torvalds@osdl.org>
Cc: "Michael S. Tsirkin" <mst@mellanox.co.il>,
	Junio C Hamano <junkio@cox.net>,
	git@vger.kernel.org
Subject: Re: tree corrupted on disk quota full
Date: Thu, 11 Jan 2007 21:17:51 +0000	[thread overview]
Message-ID: <45A6A97F.5080008@shadowen.org> (raw)
In-Reply-To: <Pine.LNX.4.64.0701111109070.3594@woody.osdl.org>

Linus Torvalds wrote:
> 
> On Thu, 11 Jan 2007, Linus Torvalds wrote:
>> That said, clearly something didn't check the error return of a write() 
>> call. Some of that got fixed up recently, so it might even be fixed in 
>> current git already.
> 
> I'm not convinced.
> 
> The "write_in_full()" logic is supposed to help people avoid problems, but 
> it *still* returns a success for a partial write.
> 
> Example of extreme breakage:
> 
> 	static int write_buffer(int fd, const void *buf, size_t len)
> 	{
> 	        ssize_t size;
> 	
> 	        size = write_in_full(fd, buf, len);
> 	        if (!size)
> 	                return error("file write: disk full");
> 	        if (size < 0)
> 	                return error("file write error (%s)", strerror(errno));
> 	        return 0;
> 	}
> 
> which is TOTAL GARBAGE, because the disk-full event might have happened in 
> the middle of the write, so "write_in_full()" might have returned 4096, 
> for example, even though the buffer length was much bigger.
> 
> I personally think write_in_full() is totally mis-designed. If you are 
> ready to handle partial writes, you should use "xwrite()". If you're not 
> ready to handle partial writes, you should either use "write_or_die()", 
> _or_ you should expect a partial write to at least return an error code 
> (which is how "write_buffer()" works).
> 
> But that's not how write_in_full() actually works. Write-in-full does not 
> return an error for a partial write, it returns the partial size.
> 
> Which is idiotic. It makes the function pointless. Just use xwrite() for 
> that.

The call was intended to replace a common idiom:

if (xwrite(fd, buf, size) != size)
	error

write_in_full() is intended as a drop in replacement for that.  On a
short write we will return a short count and that will fail such a test.
 The call basically implements the standard write() call semantic with
maximum attempts to complete.

That said returning -1 would have the same effect.

-apw

  parent reply	other threads:[~2007-01-11 21:18 UTC|newest]

Thread overview: 24+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2007-01-11 12:57 tree corrupted on disk quota full Michael S. Tsirkin
2007-01-11 13:40 ` Andreas Ericsson
2007-01-11 13:43   ` Andy Whitcroft
2007-01-11 13:59   ` Michael S. Tsirkin
2007-01-11 14:28     ` Andreas Ericsson
2007-01-11 18:31 ` Linus Torvalds
2007-01-11 19:19   ` Linus Torvalds
2007-01-11 20:03     ` Linus Torvalds
2007-01-11 21:02     ` Junio C Hamano
2007-01-11 21:17     ` Andy Whitcroft [this message]
2007-01-11 21:27       ` Junio C Hamano
2007-01-11 22:09         ` Better error messages for corrupt databases Linus Torvalds
2007-01-12  0:40           ` Shawn O. Pearce
2007-01-11 21:59       ` tree corrupted on disk quota full Linus Torvalds
2007-01-11 22:10         ` Andy Whitcroft
2007-01-11 22:32           ` Linus Torvalds
2007-01-11 21:11   ` Michael S. Tsirkin
2007-01-11 21:50     ` Linus Torvalds
2007-01-11 21:58       ` Michael S. Tsirkin
2007-01-11 22:24         ` Linus Torvalds
2007-01-11 22:39           ` Michael S. Tsirkin
2007-01-12  0:42             ` Linus Torvalds
2007-01-12  0:51               ` Shawn O. Pearce
2007-01-13  0:53           ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=45A6A97F.5080008@shadowen.org \
    --to=apw@shadowen.org \
    --cc=git@vger.kernel.org \
    --cc=junkio@cox.net \
    --cc=mst@mellanox.co.il \
    --cc=torvalds@osdl.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox