[nginx] Style: unified request method checks.
Sorin Manole
sorin.v.manole at gmail.com
Wed Nov 25 11:19:09 UTC 2015
I meant, from the diff, it looked like all single HTTP method checks were
transitioned from & to ==, while that single one was changed from == to &.
I understand now, the checks were unified with the ones that were not in
the diff (duh, I should have though about that).
Still, as a result in some places "&" is used, and in another == is. So
IMHO I don't see the point in using == for single method checks and & for
multiple in terms of unifying usage throughout the code.
But, thanks a lot for clarifying. It makes sense.
2015-11-25 13:07 GMT+02:00 Ruslan Ermilov <ru at nginx.com>:
> On Tue, Nov 24, 2015 at 11:31:16PM +0200, Sorin Manole wrote:
> > 2015-11-24 22:41 GMT+02:00 Ruslan Ermilov <ru at nginx.com>:
> >
> > > details: http://hg.nginx.org/nginx/rev/b1858fc47e3b
> > > branches:
> > > changeset: 6306:b1858fc47e3b
> > > user: Ruslan Ermilov <ru at nginx.com>
> > > date: Fri Nov 06 15:22:43 2015 +0300
> > > description:
> > > Style: unified request method checks.
> > >
> > > diffstat:
> > >
> > > src/http/modules/ngx_http_chunked_filter_module.c | 2 +-
> > > src/http/modules/ngx_http_static_module.c | 2 +-
> > > src/http/modules/ngx_http_stub_status_module.c | 2 +-
> > > src/http/ngx_http_request.c | 2 +-
> > > src/http/ngx_http_upstream.c | 2 +-
> > > 5 files changed, 5 insertions(+), 5 deletions(-)
> > >
> > > diffs (60 lines):
> > >
> > > diff -r 18428f775b2c -r b1858fc47e3b
> > > src/http/modules/ngx_http_chunked_filter_module.c
> > > --- a/src/http/modules/ngx_http_chunked_filter_module.c Mon Nov 23
> > > 12:40:19 2015 +0300
> > > +++ b/src/http/modules/ngx_http_chunked_filter_module.c Fri Nov 06
> > > 15:22:43 2015 +0300
> > > @@ -64,7 +64,7 @@ ngx_http_chunked_header_filter(ngx_http_
> > > || r->headers_out.status == NGX_HTTP_NO_CONTENT
> > > || r->headers_out.status < NGX_HTTP_OK
> > > || r != r->main
> > > - || (r->method & NGX_HTTP_HEAD))
> > > + || r->method == NGX_HTTP_HEAD)
> > > {
> > > return ngx_http_next_header_filter(r);
> > > }
> > > diff -r 18428f775b2c -r b1858fc47e3b
> > > src/http/modules/ngx_http_static_module.c
> > > --- a/src/http/modules/ngx_http_static_module.c Mon Nov 23 12:40:19
> 2015
> > > +0300
> > > +++ b/src/http/modules/ngx_http_static_module.c Fri Nov 06 15:22:43
> 2015
> > > +0300
> > > @@ -204,7 +204,7 @@ ngx_http_static_handler(ngx_http_request
> > >
> > > #endif
> > >
> > > - if (r->method & NGX_HTTP_POST) {
> > > + if (r->method == NGX_HTTP_POST) {
> > > return NGX_HTTP_NOT_ALLOWED;
> > > }
> > >
> > > diff -r 18428f775b2c -r b1858fc47e3b
> > > src/http/modules/ngx_http_stub_status_module.c
> > > --- a/src/http/modules/ngx_http_stub_status_module.c Mon Nov 23
> > > 12:40:19 2015 +0300
> > > +++ b/src/http/modules/ngx_http_stub_status_module.c Fri Nov 06
> > > 15:22:43 2015 +0300
> > > @@ -89,7 +89,7 @@ ngx_http_stub_status_handler(ngx_http_re
> > > ngx_chain_t out;
> > > ngx_atomic_int_t ap, hn, ac, rq, rd, wr, wa;
> > >
> > > - if (r->method != NGX_HTTP_GET && r->method != NGX_HTTP_HEAD) {
> > > + if (!(r->method & (NGX_HTTP_GET|NGX_HTTP_HEAD))) {
> > >
> > Since it's about the style, not really an unification I would say.
>
> What did you mean to say, I don't quite follow?
> This particular part of the change is about unification:
>
> ngx_http_autoindex_module.c: if (!(r->method &
> (NGX_HTTP_GET|NGX_HTTP_HEAD))) {
> ngx_http_empty_gif_module.c: if (!(r->method &
> (NGX_HTTP_GET|NGX_HTTP_HEAD))) {
> ngx_http_flv_module.c: if (!(r->method & (NGX_HTTP_GET|NGX_HTTP_HEAD)))
> {
> ngx_http_gzip_static_module.c: if (!(r->method &
> (NGX_HTTP_GET|NGX_HTTP_HEAD))) {
> ngx_http_index_module.c: if (!(r->method &
> (NGX_HTTP_GET|NGX_HTTP_HEAD|NGX_HTTP_POST))) {
> ngx_http_memcached_module.c: if (!(r->method &
> (NGX_HTTP_GET|NGX_HTTP_HEAD))) {
> ngx_http_mp4_module.c: if (!(r->method & (NGX_HTTP_GET|NGX_HTTP_HEAD)))
> {
> ngx_http_random_index_module.c: if (!(r->method &
> (NGX_HTTP_GET|NGX_HTTP_HEAD|NGX_HTTP_POST))) {
> ngx_http_static_module.c: if (!(r->method &
> (NGX_HTTP_GET|NGX_HTTP_HEAD|NGX_HTTP_POST))) {
> ngx_http_stub_status_module.c: if (!(r->method &
> (NGX_HTTP_GET|NGX_HTTP_HEAD))) {
>
> > > return NGX_HTTP_NOT_ALLOWED;
> > > }
> > >
> > > diff -r 18428f775b2c -r b1858fc47e3b src/http/ngx_http_request.c
> > > --- a/src/http/ngx_http_request.c Mon Nov 23 12:40:19 2015 +0300
> > > +++ b/src/http/ngx_http_request.c Fri Nov 06 15:22:43 2015 +0300
> > > @@ -1788,7 +1788,7 @@ ngx_http_process_request_header(ngx_http
> > > }
> > > }
> > >
> > > - if (r->method & NGX_HTTP_TRACE) {
> > > + if (r->method == NGX_HTTP_TRACE) {
> > > ngx_log_error(NGX_LOG_INFO, r->connection->log, 0,
> > > "client sent TRACE method");
> > > ngx_http_finalize_request(r, NGX_HTTP_NOT_ALLOWED);
> > > diff -r 18428f775b2c -r b1858fc47e3b src/http/ngx_http_upstream.c
> > > --- a/src/http/ngx_http_upstream.c Mon Nov 23 12:40:19 2015 +0300
> > > +++ b/src/http/ngx_http_upstream.c Fri Nov 06 15:22:43 2015 +0300
> > > @@ -772,7 +772,7 @@ ngx_http_upstream_cache(ngx_http_request
> > > return rc;
> > > }
> > >
> > > - if ((r->method & NGX_HTTP_HEAD) &&
> u->conf->cache_convert_head) {
> > > + if (r->method == NGX_HTTP_HEAD &&
> u->conf->cache_convert_head) {
> > > u->method = ngx_http_core_get_method;
> > > }
> > >
> > >
> > > _______________________________________________
> > > nginx-devel mailing list
> > > nginx-devel at nginx.org
> > > http://mailman.nginx.org/mailman/listinfo/nginx-devel
> > >
>
>
> --
> Ruslan Ermilov
>
> _______________________________________________
> nginx-devel mailing list
> nginx-devel at nginx.org
> http://mailman.nginx.org/mailman/listinfo/nginx-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.nginx.org/pipermail/nginx-devel/attachments/20151125/4c3e254c/attachment.html>
More information about the nginx-devel
mailing list