Re: Clean up write_in_full() users

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 




On Thu, 11 Jan 2007, Shawn O. Pearce wrote:
> 
> 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
-
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html

[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]