On Wed, Nov 21, 2018 at 03:50:30PM +0100, Miroslav Novy wrote: > Hi Ruslan, > there is my fix to remove warning "using uninitialized variable". > > I have moved the reading value of limit_rate variable from function > ngx_http_update_location_config to function ngx_http_write_filter > because value is initialized later. > > Best regards > Miroslav Nový > > # HG changeset patch > # User Miroslav Nový <miran...@gmail.com> > # Date 1542811768 0 > # Wed Nov 21 14:49:28 2018 +0000 > # Node ID e2139cd62c7263dc182b11a847a6eaf2d9560a0d > # Parent 82eed7650622fd780dcd4a86661de7b80b44199c > Variables support in limit_rate and limit_rate_after fix > > Reading value of limit_rate variable moved to function > ngx_http_write_filter because in function > ngx_http_update_location_config is not inicialized yet. > > diff -r 82eed7650622 -r e2139cd62c72 src/http/ngx_http_core_module.c > --- a/src/http/ngx_http_core_module.c Wed Nov 21 10:40:01 2018 +0000 > +++ b/src/http/ngx_http_core_module.c Wed Nov 21 14:49:28 2018 +0000 > @@ -1212,8 +1212,6 @@ > void > ngx_http_update_location_config(ngx_http_request_t *r) > { > - size_t limit_rate; > - ngx_str_t val; > ngx_http_core_loc_conf_t *clcf; > > clcf = ngx_http_get_module_loc_conf(r, ngx_http_core_module); > @@ -1283,20 +1281,6 @@ > r->connection->tcp_nopush = NGX_TCP_NOPUSH_DISABLED; > } > > - if (r->limit_rate == 0 > - && clcf->limit_rate > - && ngx_http_complex_value_size(r, clcf->limit_rate, &val, > &limit_rate) > - == NGX_OK) > - { > - if (limit_rate != (size_t) NGX_ERROR) { > - r->limit_rate = limit_rate; > - > - } else if (val.len) { > - ngx_log_error(NGX_LOG_ERR, r->connection->log, 0, > - "invalid \"limit_rate\" value \"%V\"", &val); > - } > - } > - > if (clcf->handler) { > r->content_handler = clcf->handler; > } > diff -r 82eed7650622 -r e2139cd62c72 src/http/ngx_http_write_filter_module.c > --- a/src/http/ngx_http_write_filter_module.c Wed Nov 21 10:40:01 2018 +0000 > +++ b/src/http/ngx_http_write_filter_module.c Wed Nov 21 14:49:28 2018 +0000 > @@ -48,7 +48,7 @@ > ngx_http_write_filter(ngx_http_request_t *r, ngx_chain_t *in) > { > off_t size, sent, nsent, limit; > - size_t limit_rate_after; > + ssize_t limit_rate, limit_rate_after; > ngx_str_t val; > ngx_uint_t last, flush, sync; > ngx_msec_t delay; > @@ -220,6 +220,25 @@ > return NGX_ERROR; > } > > + if (r->limit_rate == 0 > + && clcf->limit_rate > + && ngx_http_complex_value_size(r, clcf->limit_rate, &val, > + &limit_rate) > + == NGX_OK) > + { > + if (limit_rate_after != NGX_ERROR) {
You're checking the wrong variable here. > + r->limit_rate = limit_rate; > + > + } else if (val.len) { > + ngx_log_error(NGX_LOG_ERR, c->log, 0, > + "invalid \"limit_rate\" value \"%V\"", > + &val); > + } > + } > + > + ngx_log_debug1(NGX_LOG_DEBUG_HTTP, c->log, 0, "http limit rate \"%z\"", > + r->limit_rate); > + Unfortunately, moving it here from the write filter the way it's done breaks unbuffered proxying combined with limit_rate set. > if (r->limit_rate) { > if (r->limit_rate_after == 0 > && clcf->limit_rate_after > @@ -227,7 +246,7 @@ > &limit_rate_after) > == NGX_OK) > { > - if (limit_rate_after != (size_t) NGX_ERROR) { > + if (limit_rate_after != NGX_ERROR) { > r->limit_rate_after = limit_rate_after; > > } else if (val.len) { Here's my take. I didn't change size_t vs. ssize_t as I believe it's okay the way I did it. # HG changeset patch # User Ruslan Ermilov <r...@nginx.com> # Date 1544099131 -10800 # Thu Dec 06 15:25:31 2018 +0300 # Node ID 1a8c0a959f636b5eea0a4f42711af08591c23a99 # Parent 2117637f64e981e0e14c3a4b0509252fefd8a78a Added ngx_http_set_complex_value_size_slot(). If a complex value is expected to be size_t, and the compiled value is constant, the constant size_t value is remembered at compile time. The value is accessed through ngx_http_complex_value_size() which either returns the remembered constant or evaluates the expression and parses it as size_t. diff --git a/src/http/ngx_http_script.c b/src/http/ngx_http_script.c --- a/src/http/ngx_http_script.c +++ b/src/http/ngx_http_script.c @@ -105,6 +105,25 @@ ngx_http_complex_value(ngx_http_request_ ngx_int_t +ngx_http_complex_value_size(ngx_http_request_t *r, + ngx_http_complex_value_t *val, ngx_str_t *value, size_t *size) +{ + if (val->lengths == NULL) { + *size = val->u.size; + return NGX_OK; + } + + if (ngx_http_complex_value(r, val, value) != NGX_OK) { + return NGX_ERROR; + } + + *size = ngx_parse_size(value); + + return NGX_OK; +} + + +ngx_int_t ngx_http_compile_complex_value(ngx_http_compile_complex_value_t *ccv) { ngx_str_t *v; @@ -244,6 +263,36 @@ ngx_http_set_complex_value_slot(ngx_conf } +char * +ngx_http_set_complex_value_size_slot(ngx_conf_t *cf, ngx_command_t *cmd, + void *conf) +{ + char *p = conf; + + char *rv; + ngx_http_complex_value_t *cv; + + rv = ngx_http_set_complex_value_slot(cf, cmd, conf); + + if (rv != NGX_CONF_OK) { + return rv; + } + + cv = *(ngx_http_complex_value_t **) (p + cmd->offset); + + if (cv->lengths) { + return NGX_CONF_OK; + } + + cv->u.size = ngx_parse_size(&cv->value); + if (cv->u.size == (size_t) NGX_ERROR) { + return "invalid value"; + } + + return NGX_CONF_OK; +} + + ngx_int_t ngx_http_test_predicates(ngx_http_request_t *r, ngx_array_t *predicates) { diff --git a/src/http/ngx_http_script.h b/src/http/ngx_http_script.h --- a/src/http/ngx_http_script.h +++ b/src/http/ngx_http_script.h @@ -68,6 +68,10 @@ typedef struct { ngx_uint_t *flushes; void *lengths; void *values; + + union { + size_t size; + } u; } ngx_http_complex_value_t; @@ -207,9 +211,13 @@ void ngx_http_script_flush_complex_value ngx_http_complex_value_t *val); ngx_int_t ngx_http_complex_value(ngx_http_request_t *r, ngx_http_complex_value_t *val, ngx_str_t *value); +ngx_int_t ngx_http_complex_value_size(ngx_http_request_t *r, + ngx_http_complex_value_t *val, ngx_str_t *value, size_t *size); ngx_int_t ngx_http_compile_complex_value(ngx_http_compile_complex_value_t *ccv); char *ngx_http_set_complex_value_slot(ngx_conf_t *cf, ngx_command_t *cmd, void *conf); +char *ngx_http_set_complex_value_size_slot(ngx_conf_t *cf, ngx_command_t *cmd, + void *conf); ngx_int_t ngx_http_test_predicates(ngx_http_request_t *r, # HG changeset patch # User Ruslan Ermilov <r...@nginx.com> # Date 1544106977 -10800 # Thu Dec 06 17:36:17 2018 +0300 # Node ID 586277aaab1be28cf53d2caa553a2bef11be2f34 # Parent 1a8c0a959f636b5eea0a4f42711af08591c23a99 Variables support in limit_rate and limit_rate_after (ticket #293). diff --git a/src/http/ngx_http_core_module.c b/src/http/ngx_http_core_module.c --- a/src/http/ngx_http_core_module.c +++ b/src/http/ngx_http_core_module.c @@ -479,7 +479,7 @@ static ngx_command_t ngx_http_core_comm { ngx_string("limit_rate"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF |NGX_CONF_TAKE1, - ngx_conf_set_size_slot, + ngx_http_set_complex_value_size_slot, NGX_HTTP_LOC_CONF_OFFSET, offsetof(ngx_http_core_loc_conf_t, limit_rate), NULL }, @@ -487,7 +487,7 @@ static ngx_command_t ngx_http_core_comm { ngx_string("limit_rate_after"), NGX_HTTP_MAIN_CONF|NGX_HTTP_SRV_CONF|NGX_HTTP_LOC_CONF|NGX_HTTP_LIF_CONF |NGX_CONF_TAKE1, - ngx_conf_set_size_slot, + ngx_http_set_complex_value_size_slot, NGX_HTTP_LOC_CONF_OFFSET, offsetof(ngx_http_core_loc_conf_t, limit_rate_after), NULL }, @@ -1281,10 +1281,6 @@ ngx_http_update_location_config(ngx_http r->connection->tcp_nopush = NGX_TCP_NOPUSH_DISABLED; } - if (r->limit_rate == 0) { - r->limit_rate = clcf->limit_rate; - } - if (clcf->handler) { r->content_handler = clcf->handler; } @@ -3362,6 +3358,8 @@ ngx_http_core_create_loc_conf(ngx_conf_t * clcf->exact_match = 0; * clcf->auto_redirect = 0; * clcf->alias = 0; + * clcf->limit_rate = NULL; + * clcf->limit_rate_after = NULL; * clcf->gzip_proxied = 0; * clcf->keepalive_disable = 0; */ @@ -3392,8 +3390,6 @@ ngx_http_core_create_loc_conf(ngx_conf_t clcf->send_timeout = NGX_CONF_UNSET_MSEC; clcf->send_lowat = NGX_CONF_UNSET_SIZE; clcf->postpone_output = NGX_CONF_UNSET_SIZE; - clcf->limit_rate = NGX_CONF_UNSET_SIZE; - clcf->limit_rate_after = NGX_CONF_UNSET_SIZE; clcf->keepalive_timeout = NGX_CONF_UNSET_MSEC; clcf->keepalive_header = NGX_CONF_UNSET; clcf->keepalive_requests = NGX_CONF_UNSET_UINT; @@ -3581,6 +3577,14 @@ ngx_http_core_merge_loc_conf(ngx_conf_t ngx_conf_merge_msec_value(conf->client_body_timeout, prev->client_body_timeout, 60000); + if (conf->limit_rate == NULL) { + conf->limit_rate = prev->limit_rate; + } + + if (conf->limit_rate_after == NULL) { + conf->limit_rate_after = prev->limit_rate_after; + } + ngx_conf_merge_bitmask_value(conf->keepalive_disable, prev->keepalive_disable, (NGX_CONF_BITMASK_SET @@ -3622,9 +3626,6 @@ ngx_http_core_merge_loc_conf(ngx_conf_t ngx_conf_merge_size_value(conf->send_lowat, prev->send_lowat, 0); ngx_conf_merge_size_value(conf->postpone_output, prev->postpone_output, 1460); - ngx_conf_merge_size_value(conf->limit_rate, prev->limit_rate, 0); - ngx_conf_merge_size_value(conf->limit_rate_after, prev->limit_rate_after, - 0); ngx_conf_merge_msec_value(conf->keepalive_timeout, prev->keepalive_timeout, 75000); ngx_conf_merge_sec_value(conf->keepalive_header, diff --git a/src/http/ngx_http_core_module.h b/src/http/ngx_http_core_module.h --- a/src/http/ngx_http_core_module.h +++ b/src/http/ngx_http_core_module.h @@ -350,13 +350,14 @@ struct ngx_http_core_loc_conf_s { size_t client_body_buffer_size; /* client_body_buffer_size */ size_t send_lowat; /* send_lowat */ size_t postpone_output; /* postpone_output */ - size_t limit_rate; /* limit_rate */ - size_t limit_rate_after; /* limit_rate_after */ size_t sendfile_max_chunk; /* sendfile_max_chunk */ size_t read_ahead; /* read_ahead */ size_t subrequest_output_buffer_size; /* subrequest_output_buffer_size */ + ngx_http_complex_value_t *limit_rate; /* limit_rate */ + ngx_http_complex_value_t *limit_rate_after; /* limit_rate_after */ + ngx_msec_t client_body_timeout; /* client_body_timeout */ ngx_msec_t send_timeout; /* send_timeout */ ngx_msec_t keepalive_timeout; /* keepalive_timeout */ diff --git a/src/http/ngx_http_request.h b/src/http/ngx_http_request.h --- a/src/http/ngx_http_request.h +++ b/src/http/ngx_http_request.h @@ -150,6 +150,9 @@ #define NGX_HTTP_COPY_BUFFERED 0x04 +#define NGX_HTTP_LIMIT_RATE_OFF NGX_MAX_SIZE_T_VALUE + + typedef enum { NGX_HTTP_INITING_REQUEST_STATE = 0, NGX_HTTP_READING_REQUEST_STATE, diff --git a/src/http/ngx_http_upstream.c b/src/http/ngx_http_upstream.c --- a/src/http/ngx_http_upstream.c +++ b/src/http/ngx_http_upstream.c @@ -2974,7 +2974,7 @@ ngx_http_upstream_send_response(ngx_http r->write_event_handler = ngx_http_upstream_process_non_buffered_downstream; - r->limit_rate = 0; + r->limit_rate = NGX_HTTP_LIMIT_RATE_OFF; if (u->input_filter_init(u->input_filter_ctx) == NGX_ERROR) { ngx_http_upstream_finalize_request(r, u, NGX_ERROR); diff --git a/src/http/ngx_http_write_filter_module.c b/src/http/ngx_http_write_filter_module.c --- a/src/http/ngx_http_write_filter_module.c +++ b/src/http/ngx_http_write_filter_module.c @@ -48,6 +48,8 @@ ngx_int_t ngx_http_write_filter(ngx_http_request_t *r, ngx_chain_t *in) { off_t size, sent, nsent, limit; + size_t limit_rate; + ngx_str_t val; ngx_uint_t last, flush, sync; ngx_msec_t delay; ngx_chain_t *cl, *ln, **ll, *chain; @@ -250,9 +252,43 @@ ngx_http_write_filter(ngx_http_request_t return NGX_ERROR; } - if (r->limit_rate) { - if (r->limit_rate_after == 0) { - r->limit_rate_after = clcf->limit_rate_after; + if (r->limit_rate == 0) { + + if (clcf->limit_rate + && ngx_http_complex_value_size(r, clcf->limit_rate, &val, + &limit_rate) + == NGX_OK) + { + if (limit_rate != (size_t) NGX_ERROR) { + r->limit_rate = limit_rate; + + } else if (val.len) { + ngx_log_error(NGX_LOG_ERR, c->log, 0, + "invalid \"limit_rate\" value \"%V\"", &val); + } + } + + if (r->limit_rate == 0) { + r->limit_rate = NGX_HTTP_LIMIT_RATE_OFF; + } + } + + if (r->limit_rate != NGX_HTTP_LIMIT_RATE_OFF) { + + if (r->limit_rate_after == 0 + && clcf->limit_rate_after + && ngx_http_complex_value_size(r, clcf->limit_rate_after, &val, + &limit_rate) + == NGX_OK) + { + if (limit_rate != (size_t) NGX_ERROR) { + r->limit_rate_after = limit_rate; + + } else if (val.len) { + ngx_log_error(NGX_LOG_ERR, c->log, 0, + "invalid \"limit_rate_after\" value \"%V\"", + &val); + } } limit = (off_t) r->limit_rate * (ngx_time() - r->start_sec + 1) @@ -293,7 +329,7 @@ ngx_http_write_filter(ngx_http_request_t return NGX_ERROR; } - if (r->limit_rate) { + if (r->limit_rate != NGX_HTTP_LIMIT_RATE_OFF) { nsent = c->sent; _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel