Re: Clean up write_in_full() users

From: Linus Torvalds <torvalds@osdl.org>
Date: Jan 12, 2007, 04:43 UTC
Message-ID: <Pine.LNX.4.64.0701112039100.3594@woody.osdl.org>
In-Reply-To: <20070112043346.GB24195@spearce.org>

On Thu, 11 Jan 2007, Shawn O. Pearce wrote:

Show 6 quoted lines

> 
> AFAIK the only user of read_or_die is sha1_file.c when it reads in
> the 12 byte pack header and the 20 byte pack trailer to "quickly"
> verify the packfile is sane before using it.  If I recall correctly
> it was correct when I created it, but the read_in_full refactoring
> changed it.

Well, I just sent a patch to hopefully fix it, but I also think that the whole function is really misdesigned.

The thing is, "write_or_die()" actually makes sense. If you get a write error, things are really seriously broken, and it makes a lot of sense to just say "things are totally broken".

HOWEVER. The same thing is not true of a partial read. If a read doesn't succeed fully, the most _common_ case is that a file is truncated, and it does generally NOT make sense to just die and say "partial file".

So for a write error, it's ok to say "ok, I can't write, I'm dead".

For a read error, at the very least you have to say WHICH FILE couldn't be read, because it's usually a matter of some file just being too short, not some system-wide problem.

Looking at the two call-sites of "read_or_die()", they really are better off not dying, or if they died, the caller should have passed in the _reason_ (ie the name of the pack-file), or should have just used a non-dying version and tested the return value and written a message of its own.

But at least the function now WORKS after the patch I just sent out. Which it didn't do before.

			Linus