Hello!
On Fri, Mar 10, 2017 at 12:10:33PM +0300, Matwey V. Kornilov wrote:
> # HG changeset patch
> # User Matwey V. Kornilov <matwey.korni...@gmail.com>
> # Date 1486576729 -10800
> # Wed Feb 08 20:58:49 2017 +0300
> # Branch fsize
> # Node ID 35527b6bd4d84e41e030972b770e75ef583cf0f5
> # Parent a682e88cdcddb04905814cdfacde3df9ebc2b48b
> SSI: Refactor ngx_http_ssi_date_gmt_local_variable
Style, should be:
SSI: refactored ngx_http_ssi_date_gmt_local_variable().
>
> Introduce new function ngx_http_ssi_format_time.
> The function will be reused in the following commit.
>
> diff -r a682e88cdcdd -r 35527b6bd4d8
> src/http/modules/ngx_http_ssi_filter_module.c
> --- a/src/http/modules/ngx_http_ssi_filter_module.c Fri Mar 10 12:02:53
> 2017 +0300
> +++ b/src/http/modules/ngx_http_ssi_filter_module.c Wed Feb 08 20:58:49
> 2017 +0300
> @@ -110,6 +110,8 @@
> static ngx_int_t ngx_http_ssi_endblock(ngx_http_request_t *r,
> ngx_http_ssi_ctx_t *ctx, ngx_str_t **params);
>
> +static ngx_int_t ngx_http_ssi_format_time(ngx_http_request_t *r, ngx_str_t
> *v,
> + time_t time, ngx_str_t *timefmt, uintptr_t gmt);
> static ngx_int_t ngx_http_ssi_date_gmt_local_variable(ngx_http_request_t *r,
> ngx_http_variable_value_t *v, uintptr_t gmt);
>
General style rule is to place calling functions above the
functions they call. So it should be
ngx_http_ssi_date_gmt_local_variable() and then
ngx_http_ssi_format_time(). The same applies for both
declarations and definitions.
> @@ -2874,25 +2876,12 @@
>
>
> static ngx_int_t
> -ngx_http_ssi_date_gmt_local_variable(ngx_http_request_t *r,
> - ngx_http_variable_value_t *v, uintptr_t gmt)
> +ngx_http_ssi_format_time(ngx_http_request_t *r, ngx_str_t *v, time_t time,
> + ngx_str_t *timefmt, uintptr_t gmt)
> {
> - time_t now;
> - ngx_http_ssi_ctx_t *ctx;
> - ngx_str_t *timefmt;
> struct tm tm;
> char buf[NGX_HTTP_SSI_DATE_LEN];
>
> - v->valid = 1;
> - v->no_cacheable = 0;
> - v->not_found = 0;
> -
> - now = ngx_time();
> -
> - ctx = ngx_http_get_module_ctx(r, ngx_http_ssi_filter_module);
> -
> - timefmt = ctx ? &ctx->timefmt : &ngx_http_ssi_timefmt;
> -
> if (timefmt->len == sizeof("%s") - 1
> && timefmt->data[0] == '%' && timefmt->data[1] == 's')
> {
> @@ -2901,15 +2890,15 @@
> return NGX_ERROR;
> }
>
> - v->len = ngx_sprintf(v->data, "%T", now) - v->data;
> + v->len = ngx_sprintf(v->data, "%T", time) - v->data;
>
> return NGX_OK;
> }
>
> if (gmt) {
> - ngx_libc_gmtime(now, &tm);
> + ngx_libc_gmtime(time, &tm);
> } else {
> - ngx_libc_localtime(now, &tm);
> + ngx_libc_localtime(time, &tm);
> }
>
> v->len = strftime(buf, NGX_HTTP_SSI_DATE_LEN,
> @@ -2930,6 +2919,36 @@
>
>
> static ngx_int_t
> +ngx_http_ssi_date_gmt_local_variable(ngx_http_request_t *r,
> + ngx_http_variable_value_t *v, uintptr_t gmt)
> +{
> + time_t now;
> + ngx_http_ssi_ctx_t *ctx;
> + ngx_str_t *timefmt;
> + ngx_str_t timestr;
> +
> + v->valid = 1;
> + v->no_cacheable = 0;
> + v->not_found = 0;
> +
> + now = ngx_time();
> +
> + ctx = ngx_http_get_module_ctx(r, ngx_http_ssi_filter_module);
> +
> + timefmt = ctx ? &ctx->timefmt : &ngx_http_ssi_timefmt;
> +
> + if (ngx_http_ssi_format_time(r, ×tr, now, timefmt, gmt) != NGX_OK) {
> + return NGX_ERROR;
> + }
> +
> + v->data = timestr.data;
> + v->len = timestr.len;
> +
> + return NGX_OK;
> +}
Overral, I can't say I like this refactoring. A function with 5
complex arguments is not what we used to have, especially for such
simple tasks. Not to mention that ngx_str_t argument called "v"
looks at least wierd. If this needs changing, it should be done
better.
Alternatively, it may be better idea to just re-implement needed
parts in the flastmod implementation.
> +
> +
> +static ngx_int_t
> ngx_http_ssi_preconfiguration(ngx_conf_t *cf)
> {
> ngx_int_t rc;
--
Maxim Dounin
http://nginx.org/
_______________________________________________
nginx-devel mailing list
nginx-devel@nginx.org
http://mailman.nginx.org/mailman/listinfo/nginx-devel
