Hello!

On Sun, Jan 22, 2012 at 10:40:34AM +0100, Piotr Sikora wrote:

> Hi Maxim,
>
> >I tend to think this patch isn't enough, as it might not del
> >events already added as a result.
>
> You're correct, as usual.
>
> I back-ported this from the module I'm working on (with single
> ngx_add_event() call) and apparently I didn't pay enough attention.
>
> >Probably better solution would be to just return NGX_ERROR if
> >we've already set pc->connection, and let real
> >ngx_close_connection() to do the work.
>
> Yes, this would indeed work better, but to be honest, I don't see
> much point in deferring ngx_close_connection() call. Personally, I
> prefer when failing function cleans-up after itself on failure if
> possible, so I would propose this patch instead:
>
> --- src/event/ngx_event_connect.c.orig Wed Nov 25 18:03:59 2009
> +++ src/event/ngx_event_connect.c Sun Jan 22 09:22:31 2012
> @@ -159,6 +159,9 @@ ngx_event_connect_peer(ngx_peer_connection_t *pc)
> ngx_log_error(level, c->log, err, "connect() to %V failed",
> pc->name);
>
> + ngx_close_connection(c);
> + pc->connection = NULL;
> +
> return NGX_DECLINED;
> }
> }
> @@ -240,12 +243,8 @@ ngx_event_connect_peer(ngx_peer_connection_t *pc)
>
> failed:
>
> - ngx_free_connection(c);
> -
> - if (ngx_close_socket(s) == -1) {
> - ngx_log_error(NGX_LOG_ALERT, pc->log, ngx_socket_errno,
> - ngx_close_socket_n " failed");
> - }
> + ngx_close_connection(c);
> + pc->connection = NULL;
>
> return NGX_ERROR;
> }

Commited, thanks.

Maxim Dounin

_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel