Hello!
On Fri, Jan 31, 2014 at 09:38:08AM +0400, Igor Sysoev wrote:
> On Jan 31, 2014, at 8:35 , Ruslan Ermilov wrote:
>
> > Hi Piotr,
> >
> > On Thu, Jan 30, 2014 at 04:16:04PM -0800, Piotr Sikora wrote:
>
> [ ... ]
>
> > These seem questionable to me because the ioctl/fcntl calls are
> > known to report back in errno, and they're not wrapped into ngx_foo()
> > calls here.
> >
> > My own preferences are as follows.
> >
> > Use "ngx_errno" if we directly call ioctl/fcntl from the UNIX context.
> > I'd even prefer using "errno" here, but that doesn't seem to agree
> > with the current code. Still, some code uses "errno" here, and that
> > perhaps should be fixed as well.
> >
> > Use "ngx_socket_errno" for socket-related calls, including wrappers
> > such as ngx_nonblocking() known to apply to exclusively to sockets.
>
> Yes, ngx_socket_errno should used only for socket-related calls and
> only in cross-plarform parts which can be compiled on no-Unix platform
> (that is on Windows).
The src/os/unix/ currently has lots of ngx_socket_errno uses, e.g.
os/unix/ngx_recv.c, in ngx_unix_recv():
n = recv(c->fd, buf, size, 0);
...
err = ngx_socket_errno;
if (err == NGX_EAGAIN || err == NGX_EINTR) {
ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, err,
"recv() not ready");
n = NGX_AGAIN;
} else {
n = ngx_connection_error(c, err, "recv() failed");
break;
}
And this basically matches similar code in src/os/win32:
rc = WSARecv(c->fd, wsabuf, 1, &bytes, &flags, NULL, NULL);
...
err = ngx_socket_errno;
if (err == WSAEWOULDBLOCK) {
ngx_log_debug0(NGX_LOG_DEBUG_EVENT, c->log, err,
"WSARecv() not ready");
return NGX_AGAIN;
}
n = ngx_connection_error(c, err, "WSARecv() failed");
I don't think we should change this - it's good and easily
portable to other platforms. For me it looks like using
ngx_socket_errno even in non-cross-platform parts is ok and should
be preferred as long as it's in socket-related code.
--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
