Re: [PATCH 2 of 2] HTTP: removed unused r->port_start (In Elternzeit bis 7. Januar 2024)
Sehr geehrte Damen und Herren, liebe Kolleginnen und Kollegen, vielen Dank für Ihre E-Mail. Dies ist eine automatisch erstelle Antwort. Ich befinde mich derzeit in Elternzeit und bin ab dem 7. Januar 2024 wieder im Dienst. Bitte wenden Sie sich in dringenden Fällen an marcus.bi...@bsb-muenchen.de. -- Thank you for your email. This is an automatic reply. I'm currently on parental leave until Januar 7 2024. In urgent cases, please contact marcus.bi...@bsb-muenchen.de. -- Mit freundlichen Grüßen Johannes Baiter Bayerische Staatsbibliothek Digitale Bibliothek/Münchener DigitalisierungsZentrum (MDZ) Ludwigstr. 16 D-80539 München Tel.: +49 89 28638 2970 eMail: johannes.bai...@bsb-muenchen.de ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 2 of 2] HTTP: removed unused r->port_start
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
> # 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/
___
Re: [PATCH 2 of 2] HTTP: removed unused r->port_start (In Elternzeit bis 7. Januar 2024)
Sehr geehrte Damen und Herren, liebe Kolleginnen und Kollegen, vielen Dank für Ihre E-Mail. Dies ist eine automatisch erstelle Antwort. Ich befinde mich derzeit in Elternzeit und bin ab dem 7. Januar 2024 wieder im Dienst. Bitte wenden Sie sich in dringenden Fällen an marcus.bi...@bsb-muenchen.de. -- Thank you for your email. This is an automatic reply. I'm currently on parental leave until Januar 7 2024. In urgent cases, please contact marcus.bi...@bsb-muenchen.de. -- Mit freundlichen Grüßen Johannes Baiter Bayerische Staatsbibliothek Digitale Bibliothek/Münchener DigitalisierungsZentrum (MDZ) Ludwigstr. 16 D-80539 München Tel.: +49 89 28638 2970 eMail: johannes.bai...@bsb-muenchen.de ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 2 of 2] HTTP: removed unused r->port_start (In Elternzeit bis 7. Januar 2024)
Sehr geehrte Damen und Herren, liebe Kolleginnen und Kollegen, vielen Dank für Ihre E-Mail. Dies ist eine automatisch erstelle Antwort. Ich befinde mich derzeit in Elternzeit und bin ab dem 7. Januar 2024 wieder im Dienst. Bitte wenden Sie sich in dringenden Fällen an marcus.bi...@bsb-muenchen.de. -- Thank you for your email. This is an automatic reply. I'm currently on parental leave until Januar 7 2024. In urgent cases, please contact marcus.bi...@bsb-muenchen.de. -- Mit freundlichen Grüßen Johannes Baiter Bayerische Staatsbibliothek Digitale Bibliothek/Münchener DigitalisierungsZentrum (MDZ) Ludwigstr. 16 D-80539 München Tel.: +49 89 28638 2970 eMail: johannes.bai...@bsb-muenchen.de ___ nginx-devel mailing list nginx-devel@nginx.org https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 2 of 2] HTTP: removed unused r->port_start
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
# 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;
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
Re: [PATCH 2 of 2] HTTP: removed unused r->port_start
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).
>
>
> src/http/ngx_http_request.c | 3 +--
> src/http/ngx_http_request.h | 1 -
> 2 files changed, 1 insertions(+), 3 deletions(-)
>
>
> # HG changeset patch
> # User Vladimir Khomutov
> # Date 1699603821 -10800
> # Fri Nov 10 11:10:21 2023 +0300
> # Node ID 6f957e137407d8f3f7e34f413c92103004b44594
> # Parent 505e927eb7a75f0fdce4caddb4ab9d9c71c9b9c8
> HTTP: removed unused r->port_start.
>
> 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".
>
> 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.
--
Maxim Dounin
http://mdounin.ru/
___
nginx-devel mailing list
nginx-devel@nginx.org
https://mailman.nginx.org/mailman/listinfo/nginx-devel
