From 0e1d0400d86a710a698a4ddcf4d5c95bafcbb426 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 16 Aug 2016 09:59:09 +0900 Subject: [PATCH 1/2] Use whole chunk when performing huffman encoding --- lib/nghttp2_hd_huffman.c | 37 +++++++++++++++++++++++++++++++++---- 1 file changed, 33 insertions(+), 4 deletions(-) diff --git a/lib/nghttp2_hd_huffman.c b/lib/nghttp2_hd_huffman.c index 3fb0d886..8881aacb 100644 --- a/lib/nghttp2_hd_huffman.c +++ b/lib/nghttp2_hd_huffman.c @@ -64,15 +64,44 @@ static ssize_t huff_encode_sym(nghttp2_bufs *bufs, size_t *avail_ptr, code <<= 8 - (nbits & 0x7); } - /* we lose at most 3 bytes, but it is not critical in practice */ if (*avail_ptr < (nbits + 7) / 8) { - rv = nghttp2_bufs_advance(bufs); + /* slow path */ + if (nbits > 24) { + rv = nghttp2_bufs_addb(bufs, (uint8_t)(code >> 24)); + if (rv != 0) { + return rv; + } + nbits -= 8; + } + if (nbits > 16) { + rv = nghttp2_bufs_addb(bufs, (uint8_t)(code >> 16)); + if (rv != 0) { + return rv; + } + nbits -= 8; + } + if (nbits > 8) { + rv = nghttp2_bufs_addb(bufs, (uint8_t)(code >> 8)); + if (rv != 0) { + return rv; + } + nbits -= 8; + } + if (nbits == 8) { + rv = nghttp2_bufs_addb(bufs, (uint8_t)code); + if (rv != 0) { + return rv; + } + *avail_ptr = nghttp2_bufs_cur_avail(bufs); + return 8; + } + + rv = nghttp2_bufs_addb_hold(bufs, (uint8_t)code); if (rv != 0) { return rv; } *avail_ptr = nghttp2_bufs_cur_avail(bufs); - /* we assume that we at least 3 buffer space available */ - assert(*avail_ptr >= 3); + return (ssize_t)(8 - nbits); } /* fast path, since most code is less than 8 */ From 9b864380a5d51bdce19a9e2850899ef60dc51847 Mon Sep 17 00:00:00 2001 From: Tatsuhiro Tsujikawa Date: Tue, 16 Aug 2016 13:36:38 +0900 Subject: [PATCH 2/2] Use nghttp2_vec in nghttp2_hd_deflate_hd_vec This change is for the future enhancement where we loose the requirement about the chunk size for each buffer. --- lib/includes/nghttp2/nghttp2.h | 24 +++++++------- lib/nghttp2_buf.c | 38 ++++++++------------- lib/nghttp2_buf.h | 19 ++++------- lib/nghttp2_hd.c | 22 ++++++++++--- tests/nghttp2_hd_test.c | 60 ++++++++++++++++++++++++++++++---- 5 files changed, 104 insertions(+), 59 deletions(-) diff --git a/lib/includes/nghttp2/nghttp2.h b/lib/includes/nghttp2/nghttp2.h index bf814edd..207048c2 100644 --- a/lib/includes/nghttp2/nghttp2.h +++ b/lib/includes/nghttp2/nghttp2.h @@ -4603,14 +4603,15 @@ nghttp2_hd_deflate_hd(nghttp2_hd_deflater *deflater, uint8_t *buf, * @function * * Deflates the |nva|, which has the |nvlen| name/value pairs, into - * the |inlen| size of buf vector |bufsin|, each buf has length |buflen|. - * |buflens| is a |inlen| size of array, which will be used to store the writen - * size of each chunk. If one chunk is filled up, next chunk will be used. - * If |bufsin| is not large enough to store the deflated header block, - * this function fails with :enum:`NGHTTP2_ERR_INSUFF_BUFSIZE`. The - * caller should use `nghttp2_hd_deflate_bound()` to know the upper - * bound of buffer size required to deflate given header name/value - * pairs. + * the |veclen| size of buf vector |vec|. The each size of buffer + * must be set in len field of :type:`nghttp2_vec`, and this function + * assumes all buffer size is equal. The application is responsible + * to make sure that this assumption holds. If one chunk is filled + * up, next chunk will be used. If |vec| is not large enough to store + * the deflated header block, this function fails with + * :enum:`NGHTTP2_ERR_INSUFF_BUFSIZE`. The caller should use + * `nghttp2_hd_deflate_bound()` to know the upper bound of buffer size + * required to deflate given header name/value pairs. * * Once this function fails, subsequent call of this function always * returns :enum:`NGHTTP2_ERR_HEADER_COMP`. @@ -4626,11 +4627,12 @@ nghttp2_hd_deflate_hd(nghttp2_hd_deflater *deflater, uint8_t *buf, * Deflation process has failed. * :enum:`NGHTTP2_ERR_INSUFF_BUFSIZE` * The provided |buflen| size is too small to hold the output. + * :enum:`NGHTTP2_ERR_INVALID_ARGUMENT` + * The size of each buffer in |nva| is not the same. */ NGHTTP2_EXTERN ssize_t -nghttp2_hd_deflate_hd_vec(nghttp2_hd_deflater *deflater, uint8_t *const *bufsin, - size_t inlen, size_t buflen, size_t *const buflens, - const nghttp2_nv *nva, size_t nvlen); +nghttp2_hd_deflate_hd_vec(nghttp2_hd_deflater *deflater, const nghttp2_vec *vec, + size_t veclen, const nghttp2_nv *nva, size_t nvlen); /** * @function diff --git a/lib/nghttp2_buf.c b/lib/nghttp2_buf.c index bea16eb5..538df370 100644 --- a/lib/nghttp2_buf.c +++ b/lib/nghttp2_buf.c @@ -223,23 +223,26 @@ int nghttp2_bufs_wrap_init(nghttp2_bufs *bufs, uint8_t *begin, size_t len, return 0; } -int nghttp2_bufs_wrap_init2(nghttp2_bufs *bufs, uint8_t *const *bufs_in, - size_t in_len, size_t buf_len, nghttp2_mem *mem) { +int nghttp2_bufs_wrap_init2(nghttp2_bufs *bufs, const nghttp2_vec *vec, + size_t veclen, size_t chunklen, nghttp2_mem *mem) { size_t i = 0; nghttp2_buf_chain *cur_chain; nghttp2_buf_chain *head_chain; nghttp2_buf_chain **dst_chain = &head_chain; - head_chain = nghttp2_mem_malloc(mem, sizeof(nghttp2_buf_chain) * in_len); + if (veclen == 0 || chunklen == 0) { + return nghttp2_bufs_wrap_init(bufs, NULL, 0, mem); + } + + head_chain = nghttp2_mem_malloc(mem, sizeof(nghttp2_buf_chain) * veclen); if (head_chain == NULL) { return NGHTTP2_ERR_NOMEM; } - for (i = 0; i < in_len; ++i) { - uint8_t *begin = bufs_in[i]; + for (i = 0; i < veclen; ++i) { cur_chain = &head_chain[i]; cur_chain->next = NULL; - nghttp2_buf_wrap_init(&cur_chain->buf, begin, buf_len); + nghttp2_buf_wrap_init(&cur_chain->buf, vec[i].base, vec[i].len); *dst_chain = cur_chain; dst_chain = &cur_chain->next; @@ -251,10 +254,10 @@ int nghttp2_bufs_wrap_init2(nghttp2_bufs *bufs, uint8_t *const *bufs_in, bufs->head = head_chain; bufs->cur = bufs->head; - bufs->chunk_length = buf_len; - bufs->chunk_used = in_len; - bufs->max_chunk = in_len; - bufs->chunk_keep = in_len; + bufs->chunk_length = chunklen; + bufs->chunk_used = veclen; + bufs->max_chunk = veclen; + bufs->chunk_keep = veclen; return 0; } @@ -266,7 +269,6 @@ void nghttp2_bufs_wrap_free(nghttp2_bufs *bufs) { if (bufs->head) { nghttp2_mem_free(bufs->mem, bufs->head); - bufs->head = NULL; } } @@ -294,20 +296,6 @@ size_t nghttp2_bufs_len(nghttp2_bufs *bufs) { return len; } -size_t nghttp2_bufs_len_vec(nghttp2_bufs *bufs, size_t *const buflens) { - nghttp2_buf_chain *ci; - size_t len, total_len = 0; - int i = 0; - - for (ci = bufs->head; ci; ci = ci->next) { - len = nghttp2_buf_len(&ci->buf); - total_len += len; - buflens[i++] = len; - } - - return total_len; -} - static size_t bufs_avail(nghttp2_bufs *bufs) { return nghttp2_buf_avail(&bufs->cur->buf) + (bufs->chunk_length - bufs->offset) * diff --git a/lib/nghttp2_buf.h b/lib/nghttp2_buf.h index 36e8df95..2b39f17f 100644 --- a/lib/nghttp2_buf.h +++ b/lib/nghttp2_buf.h @@ -211,11 +211,11 @@ int nghttp2_bufs_wrap_init(nghttp2_bufs *bufs, uint8_t *begin, size_t len, nghttp2_mem *mem); /* - * Initializes |bufs| using supplied |in_len| size of buf vector |bufs_in|, - * each buf has length |buf_len|. - * The buffer size is fixed and no extra chunk buffer is allocated. - * In other words, max_chunk = chunk_keep = |in_len|. To free the resource allocated - * for |bufs|, use nghttp2_bufs_wrap_free(). + * Initializes |bufs| using supplied |veclen| size of buf vector + * |vec|, assuming that each buffer has length |chunklen|. The buffer + * size is fixed and no extra chunk buffer is allocated. In other + * words, max_chunk = chunk_keep = |in_len|. To free the resource + * allocated for |bufs|, use nghttp2_bufs_wrap_free(). * * This function returns 0 if it succeeds, or one of the following * negative error codes: @@ -223,8 +223,8 @@ int nghttp2_bufs_wrap_init(nghttp2_bufs *bufs, uint8_t *begin, size_t len, * NGHTTP2_ERR_NOMEM * Out of memory. */ -int nghttp2_bufs_wrap_init2(nghttp2_bufs *bufs, uint8_t *const *bufs_in, - size_t in_len, size_t buf_len, nghttp2_mem *mem); +int nghttp2_bufs_wrap_init2(nghttp2_bufs *bufs, const nghttp2_vec *vec, + size_t veclen, size_t chunklen, nghttp2_mem *mem); /* * Frees any related resource to the |bufs|. This function does not @@ -401,9 +401,4 @@ int nghttp2_bufs_next_present(nghttp2_bufs *bufs); */ size_t nghttp2_bufs_len(nghttp2_bufs *bufs); -/* - * Returns the total buffer length of |bufs|, and each buffer length in |buflens|. - */ -size_t nghttp2_bufs_len_vec(nghttp2_bufs *bufs, size_t *const buflens); - #endif /* NGHTTP2_BUF_H */ diff --git a/lib/nghttp2_hd.c b/lib/nghttp2_hd.c index acdbc874..3df36c44 100644 --- a/lib/nghttp2_hd.c +++ b/lib/nghttp2_hd.c @@ -1504,18 +1504,30 @@ ssize_t nghttp2_hd_deflate_hd(nghttp2_hd_deflater *deflater, uint8_t *buf, } ssize_t nghttp2_hd_deflate_hd_vec(nghttp2_hd_deflater *deflater, - uint8_t *const *bufsin, size_t inlen, - size_t buflen, size_t *const buflens, + const nghttp2_vec *vec, size_t veclen, const nghttp2_nv *nv, size_t nvlen) { nghttp2_bufs bufs; int rv; nghttp2_mem *mem; + size_t i; + size_t buflen; + size_t chunklen; - memset(buflens, 0, sizeof(size_t) * inlen); + if (veclen == 0) { + chunklen = 0; + } else { + for (i = 1; i < veclen; ++i) { + if (vec[0].len != vec[i].len) { + return NGHTTP2_ERR_INVALID_ARGUMENT; + } + } + + chunklen = vec[0].len; + } mem = deflater->ctx.mem; - rv = nghttp2_bufs_wrap_init2(&bufs, bufsin, inlen, buflen, mem); + rv = nghttp2_bufs_wrap_init2(&bufs, vec, veclen, chunklen, mem); if (rv != 0) { return rv; @@ -1523,7 +1535,7 @@ ssize_t nghttp2_hd_deflate_hd_vec(nghttp2_hd_deflater *deflater, rv = nghttp2_hd_deflate_hd_bufs(deflater, &bufs, nv, nvlen); - buflen = nghttp2_bufs_len_vec(&bufs, buflens); + buflen = nghttp2_bufs_len(&bufs); nghttp2_bufs_wrap_free(&bufs); diff --git a/tests/nghttp2_hd_test.c b/tests/nghttp2_hd_test.c index 0e74d752..503bbadf 100644 --- a/tests/nghttp2_hd_test.c +++ b/tests/nghttp2_hd_test.c @@ -1278,8 +1278,7 @@ void test_nghttp2_hd_deflate_hd_vec(void) { uint8_t buf[4096]; ssize_t blocklen; nghttp2_mem *mem; - uint8_t *bufsin[2]; - size_t buflens[2] = {0}; + nghttp2_vec vec[2]; size_t buflen; nghttp2_bufs bufs; nva_out out; @@ -1293,11 +1292,13 @@ void test_nghttp2_hd_deflate_hd_vec(void) { buflen = nghttp2_hd_deflate_bound(deflater, nva, ARRLEN(nva)); - bufsin[0] = &buf[0]; - bufsin[1] = &buf[buflen / 2]; + vec[0].base = &buf[0]; + vec[0].len = buflen / 2; + vec[1].base = &buf[buflen / 2]; + vec[1].len = buflen / 2; - blocklen = nghttp2_hd_deflate_hd_vec(deflater, bufsin, ARRLEN(bufsin), - buflen / 2, buflens, nva, ARRLEN(nva)); + blocklen = + nghttp2_hd_deflate_hd_vec(deflater, vec, ARRLEN(vec), nva, ARRLEN(nva)); CU_ASSERT(blocklen > 0); @@ -1314,6 +1315,53 @@ void test_nghttp2_hd_deflate_hd_vec(void) { nghttp2_hd_inflate_del(inflater); nghttp2_hd_deflate_del(deflater); nva_out_reset(&out, mem); + + /* check the case when veclen is 0 */ + nghttp2_hd_deflate_new(&deflater, 4096); + nghttp2_hd_inflate_new(&inflater); + + blocklen = nghttp2_hd_deflate_hd_vec(deflater, NULL, 0, nva, ARRLEN(nva)); + + CU_ASSERT(NGHTTP2_ERR_INSUFF_BUFSIZE == blocklen); + + nghttp2_hd_inflate_del(inflater); + nghttp2_hd_deflate_del(deflater); + + /* check the case when chunk length is 0 */ + vec[0].base = NULL; + vec[0].len = 0; + vec[1].base = NULL; + vec[1].len = 0; + + nghttp2_hd_deflate_new(&deflater, 4096); + nghttp2_hd_inflate_new(&inflater); + + blocklen = + nghttp2_hd_deflate_hd_vec(deflater, vec, ARRLEN(vec), nva, ARRLEN(nva)); + + CU_ASSERT(NGHTTP2_ERR_INSUFF_BUFSIZE == blocklen); + + nghttp2_hd_inflate_del(inflater); + nghttp2_hd_deflate_del(deflater); + + /* check the case where chunk size differs in each chunk */ + nghttp2_hd_deflate_new(&deflater, 4096); + nghttp2_hd_inflate_new(&inflater); + + buflen = nghttp2_hd_deflate_bound(deflater, nva, ARRLEN(nva)); + + vec[0].base = &buf[0]; + vec[0].len = buflen / 2; + vec[1].base = &buf[buflen / 2]; + vec[1].len = (buflen / 2) - 1; + + blocklen = + nghttp2_hd_deflate_hd_vec(deflater, vec, ARRLEN(vec), nva, ARRLEN(nva)); + + CU_ASSERT(NGHTTP2_ERR_INVALID_ARGUMENT == blocklen); + + nghttp2_hd_inflate_del(inflater); + nghttp2_hd_deflate_del(deflater); } static size_t encode_length(uint8_t *buf, uint64_t n, size_t prefix) {