Re: [Nbd] suspicious errno handling

[Wouter Verhelst <w@...112...>]

Hi Eric,

On Thu, Dec 15, 2016 at 09:35:44AM -0600, Eric Blake wrote:
> Noticed this while reviewing the extensions-write-zeroes branch: we
> probably ought to audit errno usage, as I spotted several places that
> use an undefined value of errno.  For example:
> 
> 		if(expwrite(req->from, pkg->data, req->len, client, fua)) {
> 			DEBUG("Write failed: %m");
> 			rep.error = nbd_errno(errno);
> 
> This is assuming that expwrite() left a sane errno value, and that
> DEBUG() didn't clobber it.  But a quick look at DEBUG() shows that it
> (can) call printf() (which can clobber errno), and a close look at
> expwrite() shows that there are error paths where errno is indeterminate
> (for example, expwrite() can return -1 if rawread_fully() fails, but
> rawread_fully() can call close() in between read() and returning a
> value, where close() can clobber errno).
> 
> It may be smarter to have functions return an actual error value (or
> negative error value) instead of -1 on failure, rather than relying on
> errno remaining unchanged through all the interim code.

Yes, that's a good idea. I don't think I'll be able to finish that
before the release of 3.15 (which I'm hoping I'll be able to do some
time next week), but I'll see what I can do.

(if you feel up to it, patches welcome ;-)

-- 
< ron> I mean, the main *practical* problem with C++, is there's like a dozen
       people in the world who think they really understand all of its rules,
       and pretty much all of them are just lying to themselves too.
 -- #debian-devel, OFTC, 2016-02-12