diff --git a/changes/+asdf-free.feature b/changes/+asdf-free.feature new file mode 100644 index 00000000..c52ac064 --- /dev/null +++ b/changes/+asdf-free.feature @@ -0,0 +1,10 @@ +Added ``asdf_free`` for releasing buffers that libasdf allocates on the +caller's behalf: those returned by ``asdf_write_to_mem``, +``asdf_ndarray_read_all``, ``asdf_ndarray_read_tile_ndim`` and +``asdf_ndarray_read_tile_2d``. + +It is currently just ``free()``, so existing code keeps working, but it stops +the allocator from being part of the ABI: freeing across a DLL boundary is +undefined where the library and the application link different C runtimes, and +naming ``free()`` in the contract would prevent these functions from ever +allocating differently. diff --git a/docs/Makefile.am b/docs/Makefile.am index ac5a4e24..19c2c113 100644 --- a/docs/Makefile.am +++ b/docs/Makefile.am @@ -18,6 +18,7 @@ EXTRA_DIST = \ api/asdf/extension.h.rst \ api/asdf/file.h.rst \ api/asdf/log.h.rst \ + api/asdf/util.h.rst \ api/asdf/value.h.rst \ api/asdf/yaml.h.rst \ changes.rst \ diff --git a/docs/api/asdf/util.h.rst b/docs/api/asdf/util.h.rst new file mode 100644 index 00000000..a8fe457c --- /dev/null +++ b/docs/api/asdf/util.h.rst @@ -0,0 +1,8 @@ +:tocdepth: 2 + +.. _util.h: + +asdf/util.h - Common utilities +============================== + +.. autodoc:: include/asdf/util.h diff --git a/docs/index.rst b/docs/index.rst index 335619ed..d26c6825 100644 --- a/docs/index.rst +++ b/docs/index.rst @@ -42,6 +42,7 @@ including ``asdf.h``. This in turn includes the following headers: api/asdf/core/ndarray.h api/asdf/core/datatype.h api/asdf/core/time.h + api/asdf/util.h Additional less commonly used APIs can be used by including the relevant headers. diff --git a/docs/usage/examples.rst b/docs/usage/examples.rst index dd1fef38..58df9a1f 100644 --- a/docs/usage/examples.rst +++ b/docs/usage/examples.rst @@ -112,8 +112,8 @@ the ASDF tree, as well as extract block data. Inline comments provide further e // The asdf_ndarray_read_tile_ functions copy a rectangular cutout of // the array into a buffer, converting datatype and endianness as needed. - // If you don't pass your own buffer one is allocated for you; either way - // you are responsible for freeing it. + // If you don't pass your own buffer one is allocated for you; in that + // case you are responsible for releasing it with asdf_free(). // origin and shape must have one entry per array dimension, so we size // them to the array we found. Here we take a cutout of up to 5 elements @@ -152,7 +152,7 @@ the ASDF tree, as well as extract block data. Inline comments provide further e printf("Value at center of cutout: %g\n", tile[tile_nelem / 2]); - free(tile); + asdf_free(tile); asdf_ndarray_destroy(ndarray); asdf_close(file); return 0; diff --git a/docs/usage/ndarrays.rst b/docs/usage/ndarrays.rst index d3bd1ecb..7350249b 100644 --- a/docs/usage/ndarrays.rst +++ b/docs/usage/ndarrays.rst @@ -161,16 +161,16 @@ the host's native byte order and, optionally, to a different numeric datatype: if (err == ASDF_NDARRAY_OK) { uint64_t n = asdf_ndarray_size(array); /* values[0 .. n-1] are now native-endian doubles */ - free(values); + asdf_free(values); } Passing ``NULL`` for the destination (as above, via a pointer whose value is ``NULL``) asks the library to allocate a buffer of the right size; the caller -then owns that memory and must ``free()`` it. Alternatively, pre-allocate a -buffer of `asdf_ndarray_nbytes` (or the appropriate size for the converted -type) and pass its address. Pass `ASDF_DATATYPE_SOURCE` as the destination -datatype to keep the array's original element type and only normalize byte -order. +then owns that memory and must release it with `asdf_free`. Alternatively, +pre-allocate a buffer of `asdf_ndarray_nbytes` (or the appropriate size for the +converted type) and pass its address. Pass `ASDF_DATATYPE_SOURCE` as the +destination datatype to keep the array's original element type and only +normalize byte order. Reading tiles ~~~~~~~~~~~~~ @@ -191,7 +191,7 @@ Often you only need a rectangular sub-region ("tile") of a large array. if (err == ASDF_NDARRAY_OK) { /* tile holds 25 native-endian floats in row-major order */ - free(tile); + asdf_free(tile); } The same buffer-ownership and datatype-conversion rules as diff --git a/docs/usage/writing.rst b/docs/usage/writing.rst index ec31b69a..e5169993 100644 --- a/docs/usage/writing.rst +++ b/docs/usage/writing.rst @@ -123,12 +123,12 @@ block index -- in one shot. fprintf(stderr, "write failed\n"); } /* ... use buf[0..len-1] ... */ - free(buf); + asdf_free(buf); This calls `asdf_write_to_mem`. When the first output argument ``*buf`` is ``NULL``, the library allocates a buffer large enough for the whole file and stores its address in ``*buf`` and its size in ``*len``. The caller is -responsible for freeing that buffer with ``free()``. Alternatively, you can +responsible for releasing that buffer with `asdf_free`. Alternatively, you can pre-allocate a buffer and pass a non-``NULL`` ``*buf`` with the available capacity in ``*len``; if the capacity is insufficient the output is truncated and a non-zero value is returned. @@ -207,7 +207,7 @@ result to an in-memory buffer. asdf_file_t *out = asdf_open(NULL); if (out == NULL) { fprintf(stderr, "error creating output file\n"); - free(src_data); + asdf_free(src_data); asdf_ndarray_destroy(cube); asdf_close(src); return 1; @@ -232,7 +232,7 @@ result to an in-memory buffer. uint8_t *out_data = asdf_ndarray_data_alloc(&out_cube); if (out_data == NULL) { fprintf(stderr, "out of memory\n"); - free(src_data); + asdf_free(src_data); asdf_ndarray_destroy(cube); asdf_close(src); asdf_close(out); @@ -252,7 +252,7 @@ result to an in-memory buffer. size_t len = 0; if (asdf_write_to(out, &buf, &len) != 0) { fprintf(stderr, "write failed\n"); - free(src_data); + asdf_free(src_data); asdf_ndarray_destroy(cube); asdf_close(out); asdf_close(src); @@ -262,9 +262,9 @@ result to an in-memory buffer. printf("wrote %zu-byte ASDF file to memory\n", len); /* The file owns out_cube's data now; asdf_close frees it */ - free(buf); + asdf_free(buf); asdf_close(out); - free(src_data); + asdf_free(src_data); asdf_ndarray_destroy(cube); asdf_close(src); return 0; diff --git a/include/asdf/core/ndarray.h b/include/asdf/core/ndarray.h index cae96e89..19ecf246 100644 --- a/include/asdf/core/ndarray.h +++ b/include/asdf/core/ndarray.h @@ -461,7 +461,7 @@ ASDF_EXPORT asdf_block_t *asdf_ndarray_block(asdf_ndarray_t *ndarray); * :param dst: Pointer to a destination `void *` already allocated to receive * the exact number of bytes in the source ndarray, or `NULL` to indicate * that a buffer should be allocated. In the latter case the caller is - * responsible for freeing the allocated buffer. + * responsible for releasing the allocated buffer with `asdf_free`. * :return: An `asdf_ndarray_err_t`; either `ASDF_NDARRAY_OK` if the data read * successfully; otherwise the relevant error code. */ @@ -490,7 +490,8 @@ asdf_ndarray_read_all(asdf_ndarray_t *ndarray, asdf_scalar_datatype_t dst_t, voi * :param dst: Pointer to a destination `void *` already allocated to receive * the exact number of bytes in the output tile based on shape and datatype, * or `NULL` to indicate that a buffer should be allocated. In the latter - * case the caller is responsible for freeing the allocated buffer. + * case the caller is responsible for releasing the allocated buffer with + * `asdf_free`. * :return: An `asdf_ndarray_err_t`; either `ASDF_NDARRAY_OK` if the data read * successfully; otherwise the relevant error code. */ @@ -523,7 +524,8 @@ ASDF_EXPORT asdf_ndarray_err_t asdf_ndarray_read_tile_ndim( * :param dst: Pointer to a destination `void *` already allocated to receive * the exact number of bytes in the output tile based on shape and datatype, * or `NULL` to indicate that a buffer should be allocated. In the latter - * case the caller is responsible for freeing the allocated buffer. + * case the caller is responsible for releasing the allocated buffer with + * `asdf_free`. * :return: An `asdf_ndarray_err_t`; either `ASDF_NDARRAY_OK` if the data read * successfully; otherwise the relevant error code. */ diff --git a/include/asdf/file.h b/include/asdf/file.h index 7d487fae..16dbabc3 100644 --- a/include/asdf/file.h +++ b/include/asdf/file.h @@ -313,9 +313,9 @@ ASDF_EXPORT int asdf_write_to_fp(asdf_file_t *file, FILE *fp); * read from ``*size``. If the buffer is not large enough to hold the file, * the output is truncated and a non-zero value is returned. * - * If ``*buf`` is NULL, a buffer is allocated with `malloc()` and a pointer to + * If ``*buf`` is NULL, a buffer is allocated for the caller and a pointer to * it is stored in ``*buf``; the allocated size is written to ``*size``. The - * caller is responsible for freeing the buffer with `free()`. + * caller is responsible for releasing the buffer with `asdf_free`. * * :param file: The `asdf_file_t *` to write * :param buf: Address of a ``void *`` buffer pointer (in/out) diff --git a/include/asdf/util.h b/include/asdf/util.h index 1f3058c9..3e8d9f9f 100644 --- a/include/asdf/util.h +++ b/include/asdf/util.h @@ -1,3 +1,11 @@ +/** + * .. _asdf/util.h: + * + * Common macros and utilities included by every other public header + */ + +// + #ifndef ASDF_UTIL_H #define ASDF_UTIL_H @@ -28,4 +36,22 @@ #define ASDF_DESTRUCTOR __attribute__((destructor)) +ASDF_BEGIN_DECLS + +/** + * Free a buffer that libasdf allocated on the caller's behalf + * + * Use this for buffers returned by `asdf_write_to_mem`, + * `asdf_ndarray_read_all`, `asdf_ndarray_read_tile_ndim` and + * `asdf_ndarray_read_tile_2d` when they were asked to allocate the + * destination buffer. Other pointers returned by libasdf have their own + * destructors and must not be passed here. + * + * :param ptr: The buffer to free; passing `NULL` is a no-op + */ +ASDF_EXPORT void asdf_free(void *ptr); + +ASDF_END_DECLS + + #endif /* ASDF_UTIL_H */ diff --git a/src/util.c b/src/util.c index cfbd208a..95ed2649 100644 --- a/src/util.c +++ b/src/util.c @@ -11,6 +11,11 @@ #include "util.h" +void asdf_free(void *ptr) { + free(ptr); +} + + size_t asdf_util_get_total_memory(void) { #ifndef HAVE_STATGRAB return 0; diff --git a/tests/test-emitter.c b/tests/test-emitter.c index 8053d118..b2cec124 100644 --- a/tests/test-emitter.c +++ b/tests/test-emitter.c @@ -126,7 +126,7 @@ MU_TEST(test_emitter_stream_switch) { free(buf_yaml); free(buf_blocks); - free(single_buf); + asdf_free(single_buf); return MUNIT_OK; } diff --git a/tests/test-file.c b/tests/test-file.c index 14577c2d..d94e3e08 100644 --- a/tests/test-file.c +++ b/tests/test-file.c @@ -849,6 +849,26 @@ MU_TEST(test_asdf_set_value_double_free) { } +/** `asdf_free` releases a buffer allocated by `asdf_write_to_mem`, and ignores NULL */ +MU_TEST(test_asdf_free) { + asdf_free(NULL); + + asdf_file_t *file = asdf_open(NULL); + assert_not_null(file); + assert_int(asdf_set_string0(file, "key", "value"), ==, ASDF_VALUE_OK); + + void *buf = NULL; + size_t size = 0; + assert_int(asdf_write_to_mem(file, &buf, &size), ==, 0); + assert_not_null(buf); + assert_size(size, >, 0); + asdf_close(file); + + asdf_free(buf); + return MUNIT_OK; +} + + MU_TEST_SUITE( file, MU_RUN_TEST(test_asdf_open_file), @@ -881,7 +901,8 @@ MU_TEST_SUITE( MU_RUN_TEST(write_minimal_empty_tree), MU_RUN_TEST(write_custom_tag_handle), MU_RUN_TEST(write_to_nonexistent_file), - MU_RUN_TEST(test_asdf_set_value_double_free) + MU_RUN_TEST(test_asdf_set_value_double_free), + MU_RUN_TEST(test_asdf_free) ); diff --git a/tests/test-ndarray.c b/tests/test-ndarray.c index 7fda58ea..832e9917 100644 --- a/tests/test-ndarray.c +++ b/tests/test-ndarray.c @@ -180,7 +180,7 @@ MU_TEST(ndarray_read_1d_tile_contiguous) { assert_int(err, ==, ASDF_NDARRAY_OK); assert_not_null(tile); assert_memory_equal(2 * sizeof(uint8_t), tile, expected1); - free(tile); + asdf_free(tile); asdf_ndarray_destroy(ndarray); /* Read tile from a 2-D array */ @@ -194,7 +194,7 @@ MU_TEST(ndarray_read_1d_tile_contiguous) { assert_int(err, ==, ASDF_NDARRAY_OK); assert_not_null(tile); assert_memory_equal(2 * sizeof(uint16_t), tile, expected2); - free(tile); + asdf_free(tile); asdf_ndarray_destroy(ndarray); /* Read tile from a 3-D array */ @@ -208,7 +208,7 @@ MU_TEST(ndarray_read_1d_tile_contiguous) { assert_int(err, ==, ASDF_NDARRAY_OK); assert_not_null(tile); assert_memory_equal(2 * sizeof(int32_t), tile, expected3); - free(tile); + asdf_free(tile); asdf_ndarray_destroy(ndarray); asdf_close(file); @@ -234,7 +234,7 @@ MU_TEST(test_asdf_ndarray_read_tile_2d) { assert_int(err, ==, ASDF_NDARRAY_OK); assert_not_null(tile); assert_memory_equal(4 * sizeof(uint16_t), tile, expected2); - free(tile); + asdf_free(tile); asdf_ndarray_destroy(ndarray); /* Read 2-D tile from the 1th layer of a 3-D array */ @@ -247,7 +247,7 @@ MU_TEST(test_asdf_ndarray_read_tile_2d) { assert_int(err, ==, ASDF_NDARRAY_OK); assert_not_null(tile); assert_memory_equal(4 * sizeof(int32_t), tile, expected3); - free(tile); + asdf_free(tile); asdf_ndarray_destroy(ndarray); asdf_close(file); @@ -273,7 +273,7 @@ MU_TEST(ndarray_read_3d_tile) { assert_int(err, ==, ASDF_NDARRAY_OK); assert_not_null(tile); assert_memory_equal(8 * sizeof(int32_t), tile, expected3); - free(tile); + asdf_free(tile); asdf_ndarray_destroy(ndarray); asdf_close(file); @@ -303,7 +303,7 @@ MU_TEST(ndarray_read_3d_tile) { assert_not_null(tile); \ dtype##_t expected[] = {0, 1, 2, 3, 4, 5, 6, 7}; \ assert_memory_equal(8 * sizeof(dtype##_t), tile, expected); \ - free(tile); \ + asdf_free(tile); \ asdf_ndarray_destroy(ndarray); \ } while (0) @@ -698,7 +698,7 @@ MU_TEST(ndarray_numeric_conversion) { #ifndef HAVE_FLOAT16 cleanup: #endif - free(array); + asdf_free(array); asdf_ndarray_destroy(ndarray); asdf_close(file); return MUNIT_OK; @@ -1288,7 +1288,7 @@ MU_TEST(ndarray_extension_embedded_ndarray) { asdf_test_affine_destroy(affine_in); asdf_close(file); - free(buf); + asdf_free(buf); return MUNIT_OK; }