improper check on written in non mmap path of tdb_write, tdb 1.2.9

[Rusty Russell]

On Sat, 5 Feb 2011 05:45:36 am James Caccese wrote:
> > Yep... last time I read this code I was tempted to rip out
> > the entire thing.
> > We don't create sparse files, so write failures here are
> > unlikely.  And
> > retrying seems like a forlorn hope (and as you've shown,
> > untested and buggy).
> > 
> > We basically try again to get the real errno, but unless
> > someone has some real
> > case (NFS or something weird?) I think we should just log
> > the error w/o errno
> > in that case.
> > 
> > Looks like Michael's code, so I've CC'd him.
> > 
> > Thoughts?
> > Rusty.
> > 
> 
> I'd go with the rewrite. I don't know the TDB codebase other then casual reading, but if you'd like I can put together a patch.
> 
> As for write failures, it may be my noobishness, but what do sparse files have to do with them? I was under the impression that anything from a signal, to the OS just feeling like it could result in an incomplete write, and that write / pwrite should be called in a loop until you get all the data out, or it gives an error. The code as it stands just looks like a buggy version of that idea except that it only tries once.

Not on a real file.  Try doing a 1G write on a file, and hitting ^C.  You'll
have to wait until the write is finished.  I was reminded of this (and
retested it!) recently by sfr.  (For pipes, sockets and devices, the rules
are different).

So for real files, there are only two cases I can think of which could
return partial write:
1) Disk is full, quota exceeded, etc.
2) I/O error, though in practice it might just return -1 an EIO, or
   return success.

Now, #1 can't happen in the tdb_write case, since the file isn't sparse by
design.  We're basically down to 

> Also, the pread call in tdb_read should be tried in a loop as well.
> 
> I'm not sure I understand what you're saying about the real errno though.

Hope that clarifies?
Rusty.