All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.