[PATCH 2 of 2] HTTP: removed unused r->port_start

[Maxim Dounin]

Hello!

On Wed, Nov 29, 2023 at 11:20:33AM +0300, Vladimir Homutov via nginx-devel wrote:

> On Tue, Nov 28, 2023 at 05:57:39AM +0300, Maxim Dounin wrote:
> > Hello!
> >
> > On Fri, Nov 10, 2023 at 12:11:55PM +0300, Vladimir Homutov via nginx-devel wrote:
> >
> > >
> > > It is no longer used since the refactoring in 8e5bf1bc87e2 (2008).
> >
> > Neither r->port_start nor r->port_end were ever used.
> >
> > The r->port_end is set by the parser, though it was never used by
> > the following code (and was never usable, since not copied by the
> > ngx_http_alloc_large_header_buffer() without r->port_start set).
> >
> > The 8e5bf1bc87e2 commit is completely unrelated, it is about
> > refactoring of the ngx_parse_inet_url() function, which had a
> > local variable named "port_start".
> 
> exactly, thanks for noticing.
> 
> >
> > >
> > > diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
> > > --- a/src/http/ngx_http_request.c
> > > +++ b/src/http/ngx_http_request.c
> > > @@ -1744,8 +1744,7 @@ ngx_http_alloc_large_header_buffer(ngx_h
> > >              }
> > >          }
> > >
> > > -        if (r->port_start) {
> > > -            r->port_start = new + (r->port_start - old);
> > > +        if (r->port_end) {
> > >              r->port_end = new + (r->port_end - old);
> > >          }
> > >
> > > 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
> > > @@ -597,7 +597,6 @@ struct ngx_http_request_s {
> > >      u_char                           *schema_end;
> > >      u_char                           *host_start;
> > >      u_char                           *host_end;
> > > -    u_char                           *port_start;
> > >      u_char                           *port_end;
> > >
> > >      unsigned                          http_minor:16;
> >
> > I don't think it's a good change.  Rather, we should either remove
> > both, or (eventually) fix these and provide some valid usage of
> > the port as parsed either from the request line or from the Host
> > header, similarly to the $host variable.
> >
> 
> I think that we should remove both, as unused code still needs to be
> maintained without any advantage, as this example shows.
> Restoring it will be trivial, if ever required.
> 
> 
> 

> # HG changeset patch
> # User Vladimir Khomutov <vl at wbsrv.ru>
> # Date 1701165434 -10800
> #      Tue Nov 28 12:57:14 2023 +0300
> # Node ID dacad3a9c7b8435a4c67ad2b67f261e7b4e36d8e
> # Parent  f366007dd23a6ce8e8427c1b3042781b618a2ade
> HTTP: removed unused r->port_start and r->port_end.
> 
> Neither r->port_start nor r->port_end were ever used.
> 
> The r->port_end is set by the parser, though it was never used by
> the following code (and was never usable, since not copied by the
> ngx_http_alloc_large_header_buffer() without r->port_start set).
> 
> diff --git a/src/http/ngx_http_parse.c b/src/http/ngx_http_parse.c
> --- a/src/http/ngx_http_parse.c
> +++ b/src/http/ngx_http_parse.c
> @@ -451,19 +451,16 @@ ngx_http_parse_request_line(ngx_http_req
>  
>              switch (ch) {
>              case '/':
> -                r->port_end = p;
>                  r->uri_start = p;
>                  state = sw_after_slash_in_uri;
>                  break;
>              case '?':
> -                r->port_end = p;
>                  r->uri_start = p;
>                  r->args_start = p + 1;
>                  r->empty_path_in_uri = 1;
>                  state = sw_uri;
>                  break;
>              case ' ':
> -                r->port_end = p;
>                  /*
>                   * use single "/" from request line to preserve pointers,
>                   * if request line will be copied to large client buffer
> diff --git a/src/http/ngx_http_request.c b/src/http/ngx_http_request.c
> --- a/src/http/ngx_http_request.c
> +++ b/src/http/ngx_http_request.c
> @@ -1735,11 +1735,6 @@ ngx_http_alloc_large_header_buffer(ngx_h
>              }
>          }
>  
> -        if (r->port_start) {
> -            r->port_start = new + (r->port_start - old);
> -            r->port_end = new + (r->port_end - old);
> -        }
> -
>          if (r->uri_ext) {
>              r->uri_ext = new + (r->uri_ext - old);
>          }
> 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
> @@ -597,8 +597,6 @@ struct ngx_http_request_s {
>      u_char                           *schema_end;
>      u_char                           *host_start;
>      u_char                           *host_end;
> -    u_char                           *port_start;
> -    u_char                           *port_end;
>  
>      unsigned                          http_minor:16;
>      unsigned                          http_major:16;

Looks good, pushed to http://mdounin.ru/hg/nginx/.

-- 
Maxim Dounin
http://mdounin.ru/