From 15dcb462a05dcef75b8b642186bfa85d245273e0 Mon Sep 17 00:00:00 2001 From: Graham Leggett Date: Thu, 25 Apr 2024 11:37:32 +0000 Subject: [PATCH] Change apr_buffer_mem_create, apr_buffer_str_create, apr_buffer_null_create apr_buffer_arraydup, and apr_buffer_dup to return apr_status_t, distinguishing clearly between APR_ENONMEM and APR_EINVAL. git-svn-id: https://svn.apache.org/repos/asf/apr/apr/trunk@1917324 13f79535-47bb-0310-9956-ffa450edef68 --- buffer/apr_buffer.c | 91 ++++++++++++++++++++++++++------------------ include/apr_buffer.h | 70 ++++++++++++++++++++-------------- test/testbuffer.c | 86 ++++++++++++++++++++++++++++++----------- 3 files changed, 159 insertions(+), 88 deletions(-) diff --git a/buffer/apr_buffer.c b/buffer/apr_buffer.c index 383bfd3f78..073bac0cb9 100644 --- a/buffer/apr_buffer.c +++ b/buffer/apr_buffer.c @@ -45,34 +45,37 @@ APR_DECLARE(apr_status_t) apr_buffer_mem_set(apr_buffer_t *buf, return APR_SUCCESS; } -APR_DECLARE(apr_buffer_t *) apr_buffer_mem_make(apr_pool_t *pool, +APR_DECLARE(apr_status_t) apr_buffer_mem_create(apr_buffer_t **mb, + apr_pool_t *pool, void *mem, apr_size_t len) { apr_buffer_t *buf; if (len > APR_BUFFER_MAX) { - return NULL; + return APR_EINVAL; } buf = apr_palloc(pool, sizeof(apr_buffer_t)); if (buf) { + buf->d.mem = mem; buf->size = len; buf->zero_terminated = 0; + + *mb = buf; + } + else { + return APR_ENOMEM; } - return buf; + return APR_SUCCESS; } APR_DECLARE(apr_status_t) apr_buffer_str_set(apr_buffer_t *buf, char *str, apr_ssize_t len) { - if (len > APR_BUFFER_MAX) { - return APR_EINVAL; - } - if (!str) { buf->d.str = NULL; buf->size = 0; @@ -86,9 +89,7 @@ APR_DECLARE(apr_status_t) apr_buffer_str_set(apr_buffer_t *buf, buf->zero_terminated = 1; } else { - buf->d.str = NULL; - buf->size = 0; - buf->zero_terminated = 0; + return APR_EINVAL; } } else { @@ -103,7 +104,8 @@ APR_DECLARE(apr_status_t) apr_buffer_str_set(apr_buffer_t *buf, return APR_SUCCESS; } -APR_DECLARE(apr_buffer_t *) apr_buffer_str_make(apr_pool_t *pool, +APR_DECLARE(apr_status_t) apr_buffer_str_create(apr_buffer_t **sb, + apr_pool_t *pool, char *str, apr_ssize_t len) { apr_buffer_t *buf; @@ -123,11 +125,11 @@ APR_DECLARE(apr_buffer_t *) apr_buffer_str_make(apr_pool_t *pool, zero_terminated = 1; } else { - return NULL; + return APR_EINVAL; } } else if (str[len]) { - return NULL; + return APR_EINVAL; } else { size = (apr_size_t)len; @@ -139,14 +141,30 @@ APR_DECLARE(apr_buffer_t *) apr_buffer_str_make(apr_pool_t *pool, buf->d.str = str; buf->size = size; buf->zero_terminated = zero_terminated; + + *sb = buf; + } + else { + return APR_ENOMEM; } - return buf; + return APR_SUCCESS; } -APR_DECLARE(apr_buffer_t *) apr_buffer_null_make(apr_pool_t *pool) +APR_DECLARE(apr_status_t) apr_buffer_null_create(apr_buffer_t **nb, + apr_pool_t *pool) { - return apr_pcalloc(pool, sizeof(apr_buffer_t)); + apr_buffer_t *buf; + + buf = apr_pcalloc(pool, sizeof(apr_buffer_t)); + + if (!buf) { + return APR_ENOMEM; + } + + *nb = buf; + + return APR_SUCCESS; } APR_DECLARE(apr_size_t) apr_buffer_len(const apr_buffer_t *buf) @@ -209,15 +227,18 @@ APR_DECLARE(void *) apr_buffer_pmemdup(apr_pool_t *pool, const apr_buffer_t *buf return apr_pmemdup(pool, buf->d.mem, len); } -APR_DECLARE(apr_buffer_t *) apr_buffer_arraydup(const apr_buffer_t *src, - apr_buffer_alloc alloc, void *ctx, - int nelts) +APR_DECLARE(apr_status_t) apr_buffer_arraydup(apr_buffer_t **out, + const apr_buffer_t *in, + apr_buffer_alloc alloc, void *ctx, + int nelts) { apr_buffer_t *dst = alloc(ctx, nelts * sizeof(apr_buffer_t)); - apr_buffer_t *d = dst; + const apr_buffer_t *src = in; + + *out = dst; if (!dst) { - return NULL; + return APR_ENOMEM; } int i; @@ -228,32 +249,28 @@ APR_DECLARE(apr_buffer_t *) apr_buffer_arraydup(const apr_buffer_t *src, void *mem = alloc(ctx, size); - if (mem) { - - memcpy(mem, src->d.mem, size); + if (!mem) { + return APR_ENOMEM; + } - dst->zero_terminated = src->zero_terminated; - dst->size = src->size; - dst->d.mem = mem; + memcpy(mem, src->d.mem, size); - } - else { - dst->zero_terminated = 0; - dst->size = 0; - dst->d.mem = NULL; - } + dst->zero_terminated = src->zero_terminated; + dst->size = src->size; + dst->d.mem = mem; src++; dst++; } - return d; + return APR_SUCCESS; } -APR_DECLARE(apr_buffer_t *) apr_buffer_dup(const apr_buffer_t *buf, - apr_buffer_alloc alloc, void *ctx) +APR_DECLARE(apr_status_t) apr_buffer_dup(apr_buffer_t **out, + const apr_buffer_t *in, + apr_buffer_alloc alloc, void *ctx) { - return apr_buffer_arraydup(buf, alloc, ctx, 1); + return apr_buffer_arraydup(out, in, alloc, ctx, 1); } APR_DECLARE(apr_buffer_t *) apr_buffer_cpy(apr_buffer_t *dst, diff --git a/include/apr_buffer.h b/include/apr_buffer.h index 494323250e..711c772776 100644 --- a/include/apr_buffer.h +++ b/include/apr_buffer.h @@ -44,9 +44,9 @@ extern "C" { #endif /** - * When passing a string to apr_buffer_str_make, this value can be - * passed to indicate a string with unknown length, and have apr_buffer_str_make - * compute the length automatically. + * When passing a string to apr_buffer_str_create or apr_buffer_str_set, this + * value can be passed to indicate a string with unknown length, and have + * apr_buffer_str_create and apr_buffer_str_set compute the length automatically. */ #define APR_BUFFER_STRING (-1) @@ -102,20 +102,22 @@ APR_DECLARE(apr_status_t) apr_buffer_mem_set(apr_buffer_t *buf, /** - * Make a apr_buffer_t containing a non zero terminated memory. + * Create a apr_buffer_t containing non zero terminated memory. * * The buffer structure is allocated from the pool, while the contents are * stored as is. It is the responsibility of the caller to ensure the * contents have a lifetime as long as the pool. + * @param mb The memory buffer returned * @param pool The pool to allocate from * @param mem The memory to assign to the buffer * @param len The length of the memory - * @return The apr_buffer_t we just made. Returns NULL if we could not - * allocate enough memory, or if len overflows. + * @return Returns APR_ENOMEM if we could not allocate enough memory, + * APR_EINVAL if len overflows, otherwise APR_SUCCESS. */ -APR_DECLARE(apr_buffer_t *) apr_buffer_mem_make(apr_pool_t *pool, +APR_DECLARE(apr_status_t) apr_buffer_mem_create(apr_buffer_t **mb, + apr_pool_t *pool, void *mem, apr_size_t len) - __attribute__((nonnull(1))); + __attribute__((nonnull(1,2))); /** * Set a apr_buffer_t with a zero terminated string. @@ -131,32 +133,36 @@ APR_DECLARE(apr_status_t) apr_buffer_str_set(apr_buffer_t *buf, __attribute__((nonnull(1))); /** - * Make a apr_buffer_t containing a zero terminated string. + * Create a apr_buffer_t containing a zero terminated string. * * The buffer structure is allocated from the pool, while the contents are * stored as is. It is the responsibility of the caller to ensure the * contents have a lifetime as long as the pool. + * @param sb The string buffer returned * @param pool The pool to allocate from. * @param str The string to assign to the buffer. * @param len The length of the string, or APR_BUFFER_STRING to have the length * calculated. - * @return The apr_buffer_t we just made. Returns NULL if we could not - * allocate memory, or if len overflows. + * @return Returns APR_ENOMEM if we could not allocate enough memory, + * APR_EINVAL if len overflows, otherwise APR_SUCCESS. */ -APR_DECLARE(apr_buffer_t *) apr_buffer_str_make(apr_pool_t *pool, +APR_DECLARE(apr_status_t) apr_buffer_str_create(apr_buffer_t **sb, + apr_pool_t *pool, char *str, apr_ssize_t len) __attribute__((nonnull(1))); /** - * Make a apr_buffer_t containing a NULL payload. + * Create a apr_buffer_t containing a NULL payload. * * The buffer structure is allocated from the pool. + * @param nb The null buffer returned * @param pool The pool to allocate from. - * @return The apr_buffer_t we just made. Returns NULL if we could not - * allocate memory. + * @return Returns APR_ENOMEM if we could not allocate enough memory, + * otherwise APR_SUCCESS. */ -APR_DECLARE(apr_buffer_t *) apr_buffer_null_make(apr_pool_t *pool) - __attribute__((nonnull(1))); +APR_DECLARE(apr_status_t) apr_buffer_null_create(apr_buffer_t **nb, + apr_pool_t *pool) + __attribute__((nonnull(1))); /** @@ -295,16 +301,20 @@ typedef void *(*apr_buffer_alloc)(void *ctx, apr_size_t size); * * Use this function to copy buffers, and the contents of the buffers, into and * out of a cache. - * @param buf The string/memory buffer. + * @param out The duplicated buffer array. If APR_ENOMEM is returned the + * array may be partially duplicated, it is up to the caller to free any + * memory allocated, and to pass zeroed buffers if needed. + * @param in The string/memory buffer. * @param alloc The function callback to allocate memory for the buffer * @param ctx Context to pass to the callback function * @param nelts Number of buffers to duplicate - * @return The array of duplicated buffers. + * @return APR_ENONMEM if the alloc function returned NULL, otherwise APR_SUCCESS */ -APR_DECLARE(apr_buffer_t *) apr_buffer_arraydup(const apr_buffer_t *buf, - apr_buffer_alloc alloc, void *ctx, - int nelts) - __attribute__((nonnull(1,2))); +APR_DECLARE(apr_status_t) apr_buffer_arraydup(apr_buffer_t **out, + const apr_buffer_t *in, + apr_buffer_alloc alloc, void *ctx, + int nelts) + __attribute__((nonnull(1,2))); /** * Return a copy of a string/memory buffer. @@ -315,14 +325,18 @@ APR_DECLARE(apr_buffer_t *) apr_buffer_arraydup(const apr_buffer_t *buf, * Use this function to copy a buffer, and the content of the buffer, into and * out of a cache. * - * @param buf The string/memory buffer. + * @param out The duplicated buffer. If APR_ENOMEM is returned the buffer may + * be partially duplicated, it is up to the caller to free any memory + * allocated, and to pass zeroed buffers if needed. + * @param in The string/memory buffer. * @param alloc The function callback to allocate memory for the buffer * @param ctx Context to pass to the callback function - * @return The duplicated buffer. + * @return APR_ENONMEM if the alloc function returned NULL, otherwise APR_SUCCESS */ -APR_DECLARE(apr_buffer_t *) apr_buffer_dup(const apr_buffer_t *buf, - apr_buffer_alloc alloc, void *ctx) - __attribute__((nonnull(1,2))); +APR_DECLARE(apr_status_t) apr_buffer_dup(apr_buffer_t **out, + const apr_buffer_t *in, + apr_buffer_alloc alloc, void *ctx) + __attribute__((nonnull(1,2))); /** * Copy the contents a buffer into another buffer. diff --git a/test/testbuffer.c b/test/testbuffer.c index c79629ccf3..1453ae5ea8 100644 --- a/test/testbuffer.c +++ b/test/testbuffer.c @@ -32,9 +32,11 @@ static void test_memory_buffer(abts_case *tc, void *data) apr_pool_create(&pool, p); - mb = apr_buffer_mem_make(pool, test_memory, sizeof(test_memory)); + ABTS_ASSERT(tc, "mem_create was not created", + APR_SUCCESS == apr_buffer_mem_create(&mb, pool, test_memory, + sizeof(test_memory))); - ABTS_ASSERT(tc, "mem_make stored wrong data", + ABTS_ASSERT(tc, "mem_create stored wrong data", memcmp(test_memory, mb->d.mem, sizeof(test_memory)) == 0); ABTS_ASSERT(tc, "memory buffer is NULL", @@ -77,21 +79,23 @@ static void test_string_buffer(abts_case *tc, void *data) apr_pool_create(&pool, p); - sb = apr_buffer_str_make(pool, test_string, APR_BUFFER_STRING); + ABTS_ASSERT(tc, "str_create was not created", + APR_SUCCESS == apr_buffer_str_create(&sb, pool, test_string, + APR_BUFFER_STRING)); - ABTS_ASSERT(tc, "str_make stored wrong data", + ABTS_ASSERT(tc, "str_create stored wrong data", strncmp(test_string, sb->d.str, strlen(test_string)) == 0); - ABTS_ASSERT(tc, "str_make's buffer is NULL", + ABTS_ASSERT(tc, "str_create's buffer is NULL", !apr_buffer_is_null(sb)); - ABTS_ASSERT(tc, "str_make's buffer is not a string", + ABTS_ASSERT(tc, "str_create's buffer is not a string", apr_buffer_is_str(sb)); - ABTS_ASSERT(tc, "str_make's buffer didn't return a string", + ABTS_ASSERT(tc, "str_create's buffer didn't return a string", apr_buffer_str(sb) != NULL); - ABTS_ASSERT(tc, "str_make's buffer returned memory", + ABTS_ASSERT(tc, "str_create's buffer returned memory", apr_buffer_mem(sb, NULL) != NULL); str = apr_buffer_pstrdup(pool, sb); @@ -115,6 +119,42 @@ static void test_string_buffer(abts_case *tc, void *data) apr_pool_destroy(pool); } +static void test_null_buffer(abts_case *tc, void *data) +{ + apr_pool_t *pool; + apr_buffer_t *nb; + + apr_pool_create(&pool, p); + + ABTS_ASSERT(tc, "null_create was not created", + APR_SUCCESS == apr_buffer_null_create(&nb, pool)); + + ABTS_ASSERT(tc, "null buffer isn't NULL", + apr_buffer_is_null(nb)); + + apr_buffer_str_set(nb, test_string, strlen(test_string)); + + ABTS_ASSERT(tc, "string buffer is NULL", + !apr_buffer_is_null(nb)); + + apr_buffer_mem_set(nb, test_memory, sizeof(test_memory)); + + ABTS_ASSERT(tc, "memory buffer is NULL", + !apr_buffer_is_null(nb)); + + apr_buffer_str_set(nb, NULL, 0); + + ABTS_ASSERT(tc, "null buffer isn't NULL", + apr_buffer_is_null(nb)); + + apr_buffer_mem_set(nb, NULL, 0); + + ABTS_ASSERT(tc, "null buffer isn't NULL", + apr_buffer_is_null(nb)); + + apr_pool_destroy(pool); +} + static void *test_buffers_palloc(void *ctx, apr_size_t size) { apr_pool_t *pool = ctx; @@ -143,7 +183,7 @@ static void test_buffers(abts_case *tc, void *data) /* duplicate the source buffers, allocating memory from a pool */ vals = apr_array_make(pool, 2, sizeof(apr_buffer_t)); - vals->elts = (char *)apr_buffer_arraydup(src, test_buffers_palloc, pool, 2); + apr_buffer_arraydup((apr_buffer_t **)(&vals->elts), src, test_buffers_palloc, pool, 2); vals->nelts = 2; dst = apr_array_pop(vals); @@ -185,37 +225,36 @@ static void test_compare_buffers(abts_case *tc, void *data) ABTS_ASSERT(tc, "NULL equals NULL", !apr_buffer_cmp(small, large)); - small = apr_buffer_null_make(pool); + apr_buffer_null_create(&small, pool); ABTS_ASSERT(tc, "null buffer equals NULL", !apr_buffer_cmp(small, large)); - large = apr_buffer_null_make(pool); - ABTS_ASSERT(tc, "null buffer equals null buffer", - !apr_buffer_cmp(small, large)); - - small = NULL; ABTS_ASSERT(tc, "NULL equals null buffer", + !apr_buffer_cmp(large, small)); + + apr_buffer_null_create(&large, pool); + ABTS_ASSERT(tc, "null buffer equals null buffer", !apr_buffer_cmp(small, large)); - small = apr_buffer_str_make(pool, same, APR_BUFFER_STRING); - large = apr_buffer_str_make(pool, same, APR_BUFFER_STRING); + apr_buffer_str_set(small, same, APR_BUFFER_STRING); + apr_buffer_str_set(large, same, APR_BUFFER_STRING); ABTS_ASSERT(tc, "pointer equals same pointer", !apr_buffer_cmp(small, large)); - small = apr_buffer_str_make(pool, "same", APR_BUFFER_STRING); - large = apr_buffer_str_make(pool, "same", APR_BUFFER_STRING); + apr_buffer_str_set(small, "same", APR_BUFFER_STRING); + apr_buffer_str_set(large, "same", APR_BUFFER_STRING); ABTS_ASSERT(tc, "'same' equals 'same'", !apr_buffer_cmp(small, large)); - small = apr_buffer_str_make(pool, "short", APR_BUFFER_STRING); - large = apr_buffer_str_make(pool, "l o n g", APR_BUFFER_STRING); + apr_buffer_str_set(small, "short", APR_BUFFER_STRING); + apr_buffer_str_set(large, "l o n g", APR_BUFFER_STRING); ABTS_ASSERT(tc, "'short' less than 'l o n g'", apr_buffer_cmp(small, large) < 0); ABTS_ASSERT(tc, "'l o n g' greater than 'short'", apr_buffer_cmp(large, small) > 0); - small = apr_buffer_str_make(pool, "aardvark", APR_BUFFER_STRING); - large = apr_buffer_str_make(pool, "zucchini", APR_BUFFER_STRING); + apr_buffer_str_set(small, "aardvark", APR_BUFFER_STRING); + apr_buffer_str_set(large, "zucchini", APR_BUFFER_STRING); ABTS_ASSERT(tc, "'aardvark' less than 'zucchini'", apr_buffer_cmp(small, large) < 0); ABTS_ASSERT(tc, "'zucchini' greater than 'aardvark'", @@ -230,6 +269,7 @@ abts_suite *testbuffer(abts_suite *suite) abts_run_test(suite, test_memory_buffer, NULL); abts_run_test(suite, test_string_buffer, NULL); + abts_run_test(suite, test_null_buffer, NULL); abts_run_test(suite, test_buffers, NULL); abts_run_test(suite, test_compare_buffers, NULL);