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/ _______________________________________________ nginx-devel mailing list nginx-devel@nginx.org http://mailman.nginx.org/mailman/listinfo/nginx-devel