[PATCH] Nullify pc->connection in case of failure

[Maxim Dounin]

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