eio.c bug - use of errno without testing result

[Micheil Smith]

Hi Marc,

Actually, I read the wrong documentation. So, in node, we currently have a pretty 
much global After callback, which does:

	static int After(eio_req *req) {
	  // ...
	  ev_unref(EV_DEFAULT_UC);
	  // ...
	  if (req->errorno != 0) {
	     // throw error
	  } else {
	     // call JS function as callback.
	  }
	  // ...
	}

Are you suggesting we should actually be doing something like:

	static int After(eio_req *req) {
	  // ...
	  ev_unref(EV_DEFAULT_UC);
	  // ...
	  if (req->result != EXPECTED_RESULT) {
	     // throw error
	  } else {
	     // call JS function as callback.
	  }
	  // ...
	}

Where EXPECTED_RESULT is the result int that we would expect given req->type?

Yours,
Micheil Smith
--
BrandedCode.com

On 30/10/2010, at 12:40 AM, Micheil Smith wrote:

> Hi Marc,
> 
> Given something like a call to EIO_OPEN, how should we be checking for an error, 
> would I be right in thinking that we should look at ev_set_syserr_cb?
> 
> because if error handling can be improved, I'd be all for learning how to improve it.
> 
> Thanks,
> Micheil Smith
> --
> BrandedCode.com
> 
> On 30/10/2010, at 12:30 AM, Marc Lehmann wrote:
> 
>> On Fri, Oct 29, 2010 at 02:40:51AM -0700, Liam <libev at networkimprov.net> wrote:
>>> The issue is... because sendfile(2) on Linux and FreeBSD succeed but set an
>>> erroneous errno, an app which simply checks the eio req->errorno to decide
>>> if there's an error may fail. Node.js does this as it provides callbacks
>>> with either an error or data, never both.
>> 
>> Then node.js is horribly broken on any OS, with any syscall, not just
>> sendfile.
>> 
>>> Since eio is supports multiple OSes, one might expect it to hide OS
>>> eccentricities?
>> 
>> It works like any other syscall actually, no syscall clears errno, node.js is
>> simply broken if it uses errno to test for failure.
>> 
>> To test for failure, one has to check the return value of the call. errno
>> only tells you _how_ something failed, not _whether_ something failed.
>> 
>> And that's the same for any syscall on any OS.
>> 
>> It would be rather eccentric if it did, in fact :)
>> 
>> -- 
>>               The choice of a       Deliantra, the free code+content MORPG
>>     -----==-     _GNU_              http://www.deliantra.net
>>     ----==-- _       generation
>>     ---==---(_)__  __ ____  __      Marc Lehmann
>>     --==---/ / _ \/ // /\ \/ /      schmorp at schmorp.de
>>     -=====/_/_//_/\_,_/ /_/\_\
>> 
>> _______________________________________________
>> libev mailing list
>> libev at lists.schmorp.de
>> http://lists.schmorp.de/cgi-bin/mailman/listinfo/libev
>