[PATCH 1 of 3] Added support for trailers in HTTP responses

[Maxim Dounin]

Hello!

On Mon, Jun 05, 2017 at 09:56:03PM -0700, Piotr Sikora via nginx-devel wrote:

> Hey Maxim,
> 
> > I would prefer to preserve the typical code path (when there are no
> > trailers) without an extra allocation.  It looks like it would be
> > as trivail as:
> >
> > @@ -273,14 +273,18 @@ ngx_http_chunked_create_trailers(ngx_htt
> >      b->memory = 1;
> >      b->last_buf = 1;
> >
> > +    if (len == sizeof(CRLF "0" CRLF CRLF) - 1) {
> > +        b->pos = (u_char *) CRLF "0" CRLF CRLF;
> > +        b->last = b->pos + sizeof(CRLF "0" CRLF CRLF) - 1;
> > +        return cl;
> > +    }
> 
> Sounds good, but the if statement reads a bit weird.
> 
> What about this instead, even though it might be a bit more expensive?
> 
> @@ -236,7 +236,7 @@ ngx_http_chunked_create_trailers(ngx_http_request_t *r,
>      ngx_list_part_t  *part;
>      ngx_table_elt_t  *header;
> 
> -    len = sizeof(CRLF "0" CRLF CRLF) - 1;
> +    len = 0;
> 
>      part = &r->headers_out.trailers.part;
>      header = part->elts;
> @@ -273,12 +273,14 @@ ngx_http_chunked_create_trailers(ngx_http_request_t *r,
>      b->memory = 1;
>      b->last_buf = 1;
> 
> -    if (len == sizeof(CRLF "0" CRLF CRLF) - 1) {
> +    if (len == 0) {
>          b->pos = (u_char *) CRLF "0" CRLF CRLF;
>          b->last = b->pos + sizeof(CRLF "0" CRLF CRLF) - 1;
>          return cl;
>      }
> 
> +    len += sizeof(CRLF "0" CRLF CRLF) - 1;
> +
>      b->pos = ngx_palloc(r->pool, len);
>      if (b->pos == NULL) {
>          return NULL;

I've tried this as well, and decided that "if (len == 
sizeof(...))" is slightly more readable, and also produces smaller 
patch to your code.   No strict preference though, feel free to 
use any variant you think is better.

-- 
Maxim Dounin
http://nginx.org/