Hello! On Wed, Feb 10, 2016 at 12:09:09AM +0600, Pavel V. Rochnyack wrote:
> # HG changeset patch > # User Pavel V. Rochnyack <[email protected]> > # Date 1454835814 -21600 > # Node ID b4748ebdd06ba79aa27e0c54fc1d627d13966bed > # Parent 3577c021f21eb4de6d09c1d624ba77ee9ee1eb6d > Limit conn: Added "off" parameter to the 'limit_conn' directive. Please do not capitalize "Added". Please do not use different quotes. Something like Limit conn: added "limit_conn off". should be good enough. > > Added support for inherited "limit_conn" directives disable. This probably can be safely omitted. Alternatively, please rewrite to something more readable, e.g.: The "limit_conn" directive with the "off" parameter allows to disable inheritance of limits from the previous level. > > diff -r 3577c021f21e -r b4748ebdd06b > src/http/modules/ngx_http_limit_conn_module.c > --- a/src/http/modules/ngx_http_limit_conn_module.c Fri Feb 05 21:48:25 > 2016 +0300 > +++ b/src/http/modules/ngx_http_limit_conn_module.c Sun Feb 07 15:03:34 > 2016 +0600 > @@ -40,6 +40,7 @@ typedef struct { > ngx_array_t limits; > ngx_uint_t log_level; > ngx_uint_t status_code; > + ngx_uint_t off; /* unsigned off:1 */ > } ngx_http_limit_conn_conf_t; > > > @@ -82,7 +83,7 @@ static ngx_command_t ngx_http_limit_con > NULL }, > > { ngx_string("limit_conn"), > - NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE2, > + NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_CONF_TAKE12, > ngx_http_limit_conn, > NGX_HTTP_LOC_CONF_OFFSET, > 0, > @@ -160,6 +161,10 @@ ngx_http_limit_conn_handler(ngx_http_req > lccf = ngx_http_get_module_loc_conf(r, ngx_http_limit_conn_module); > limits = lccf->limits.elts; > > + if (lccf->off) { > + return NGX_DECLINED; > + } > + This check should not be needed once configuration parsing properly implemented, as lccf->limits.nelts below will be 0 and the for() loop will do nothing. > for (i = 0; i < lccf->limits.nelts; i++) { > ctx = limits[i].shm_zone->data; > > @@ -466,6 +471,7 @@ ngx_http_limit_conn_create_conf(ngx_conf > > conf->log_level = NGX_CONF_UNSET_UINT; > conf->status_code = NGX_CONF_UNSET_UINT; > + conf->off = NGX_CONF_UNSET_UINT; > > return conf; > } > @@ -477,7 +483,9 @@ ngx_http_limit_conn_merge_conf(ngx_conf_ > ngx_http_limit_conn_conf_t *prev = parent; > ngx_http_limit_conn_conf_t *conf = child; > > - if (conf->limits.elts == NULL) { > + ngx_conf_merge_uint_value(conf->off, prev->off, 0); > + > + if (conf->off == 0 && conf->limits.elts == NULL) { > conf->limits = prev->limits; > } > > @@ -603,6 +611,35 @@ ngx_http_limit_conn(ngx_conf_t *cf, ngx_ > > value = cf->args->elts; > > + ngx_conf_merge_uint_value(lccf->off, lccf->off, 0); Merging the value with itself looks like a dirty hack. Correct solution would be to set conf->off to 0 in create_conf, and don't try to merge it neither here nor in merge_conf. > + > + if (ngx_strcmp(value[1].data, "off") == 0) { > + if (cf->args->nelts != 2) { > + return "has invalid number of arguments"; > + } > + > + if (lccf->off || lccf->limits.elts) { > + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, > + "\"limit_conn off\" cannot be used with other > " > + "\"limit_conn\" directives on the same > level"); > + return NGX_CONF_ERROR; > + } > + > + lccf->off = 1; > + return NGX_CONF_OK; > + } > + > + if (cf->args->nelts != 3) { > + return "has invalid number of arguments"; > + } > + > + if (lccf->off) { > + ngx_conf_log_error(NGX_LOG_EMERG, cf, 0, > + "\"limit_conn off\" cannot be used with other " > + "\"limit_conn\" directives on the same level"); > + return NGX_CONF_ERROR; > + } > + This looks overcomplicated and far from what nginx normally prints on similar errors at the same time. It should be possible to do this better. > shm_zone = ngx_shared_memory_add(cf, &value[1], 0, > &ngx_http_limit_conn_module); > if (shm_zone == NULL) { -- Maxim Dounin http://nginx.org/ p.s. Not --reply-to, but --in-reply-to. _______________________________________________ nginx-devel mailing list [email protected] http://mailman.nginx.org/mailman/listinfo/nginx-devel
