Skip to content

Commit

Permalink
Refactor to be a flag to put
Browse files Browse the repository at this point in the history
  • Loading branch information
benjeffery authored and mergify[bot] committed Jan 25, 2022
1 parent 288c6a1 commit 6f95d00
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 79 deletions.
8 changes: 8 additions & 0 deletions c/CHANGELOG.rst
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
--------------------
[2.1.0] - 2022-01-25
--------------------

- Add flag KAS_BORROWS_ARRAY to put. When specified kastore will not copy
or free the array, which must persist for the life of the store.
(:user:`benjeffery`, :pr:`181`, :issue:`180`).

--------------------
[2.0.1] - 2021-11-26
--------------------
Expand Down
126 changes: 64 additions & 62 deletions c/kastore.c
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,9 @@ kas_strerror(int err)
ret = "Bad open mode; must be \"r\", \"w\", or \"a\"";
break;
case KAS_ERR_BAD_FLAGS:
ret = "Unknow flags specified. Only (KAS_GET_TAKES_OWNERSHIP, "
"KAS_READ_ALL or KAS_PUT_BORROWS_ARRAY) or 0 can be specified";
ret = "Unknown flags specified. Only (KAS_GET_TAKES_OWNERSHIP, "
"KAS_READ_ALL or ) or 0 can be specified "
"for open, and KAS_BORROWS_ARRAY or 0 for put";
break;
case KAS_ERR_NO_MEMORY:
ret = "Out of memory";
Expand Down Expand Up @@ -341,6 +342,7 @@ kastore_write_data(kastore_t *self)
int ret = 0;
size_t j, size, offset, padding;
char pad[KAS_ARRAY_ALIGN] = { 0, 0, 0, 0, 0, 0, 0 };
const void *write_array;

offset = KAS_HEADER_SIZE + self->num_items * KAS_ITEM_DESCRIPTOR_SIZE;

Expand All @@ -362,17 +364,13 @@ kastore_write_data(kastore_t *self)
goto out;
}
size = self->items[j].array_len * type_size(self->items[j].type);
if (self->flags & KAS_PUT_BORROWS_ARRAY) {
if (size > 0
&& fwrite(self->items[j].borrowed_array, size, 1, self->file) != 1) {
ret = KAS_ERR_IO;
goto out;
}
} else {
if (size > 0 && fwrite(self->items[j].array, size, 1, self->file) != 1) {
ret = KAS_ERR_IO;
goto out;
}
write_array = self->items[j].borrowed_array != NULL
? self->items[j].borrowed_array
: self->items[j].array;
assert(write_array != NULL);
if (size > 0 && fwrite(write_array, size, 1, self->file) != 1) {
ret = KAS_ERR_IO;
goto out;
}
offset = self->items[j].array_start + size;
}
Expand Down Expand Up @@ -535,13 +533,8 @@ kastore_insert_all(kastore_t *self, kastore_t *other)

for (j = 0; j < other->num_items; j++) {
item = other->items[j];
if (other->flags && KAS_PUT_BORROWS_ARRAY) {
ret = kastore_put(
self, item.key, item.key_len, item.array, item.array_len, item.type, 0);
} else {
ret = kastore_put(self, item.key, item.key_len, item.borrowed_array,
item.array_len, item.type, 0);
}
ret = kastore_put(
self, item.key, item.key_len, item.array, item.array_len, item.type, 0);
if (ret != 0) {
goto out;
}
Expand Down Expand Up @@ -632,8 +625,7 @@ kastore_openf(kastore_t *self, FILE *file, const char *mode, int flags)
goto out;
}

if (flags > (KAS_READ_ALL | KAS_GET_TAKES_OWNERSHIP | KAS_PUT_BORROWS_ARRAY)
|| flags < 0) {
if (flags > (KAS_READ_ALL | KAS_GET_TAKES_OWNERSHIP) || flags < 0) {
ret = KAS_ERR_BAD_FLAGS;
goto out;
}
Expand Down Expand Up @@ -844,39 +836,6 @@ kastore_gets_float64(kastore_t *self, const char *key, double **array, size_t *a
return kastore_gets_type(self, key, (void **) array, array_len, KAS_FLOAT64);
}

int KAS_WARN_UNUSED
kastore_put(kastore_t *self, const char *key, size_t key_len, const void *array,
size_t array_len, int type, int flags)
{
int ret;
size_t array_size;
void *array_copy = NULL;

if (type < 0 || type >= KAS_NUM_TYPES) {
ret = KAS_ERR_BAD_TYPE;
goto out;
}
if (self->flags & KAS_PUT_BORROWS_ARRAY) {
ret = kastore_bput(self, key, key_len, array, array_len, type, flags);
} else {
array_size = type_size(type) * array_len;
array_copy = malloc(array_size == 0 ? 1 : array_size);
if (array_copy == NULL) {
ret = KAS_ERR_NO_MEMORY;
goto out;
}
memcpy(array_copy, array, array_size);
ret = kastore_oput(self, key, key_len, array_copy, array_len, type, flags);
if (ret == 0) {
/* Kastore has taken ownership of the array, so we don't need to free it */
array_copy = NULL;
}
}
out:
kas_safe_free(array_copy);
return ret;
}

static int KAS_WARN_UNUSED
kastore_put_item(kastore_t *self, kaitem_t **ret_item, const char *key, size_t key_len,
int type, int KAS_UNUSED(flags))
Expand Down Expand Up @@ -934,39 +893,82 @@ kastore_put_item(kastore_t *self, kaitem_t **ret_item, const char *key, size_t k
goto out;
}
}
out:
*ret_item = new_item;
out:
return ret;
}

int KAS_WARN_UNUSED
kastore_oput(kastore_t *self, const char *key, size_t key_len, void *array,
static int KAS_WARN_UNUSED
kastore_bput(kastore_t *self, const char *key, size_t key_len, const void *array,
size_t array_len, int type, int flags)
{
int ret = 0;
kaitem_t *item;

ret = kastore_put_item(self, &item, key, key_len, type, flags);
if (ret != 0) {
goto out;
}
item->array = array;
item->borrowed_array = array;
item->array_len = array_len;
out:
return ret;
}

int KAS_WARN_UNUSED
kastore_bput(kastore_t *self, const char *key, size_t key_len, const void *array,
kastore_put(kastore_t *self, const char *key, size_t key_len, const void *array,
size_t array_len, int type, int flags)
{
int ret;
size_t array_size;
void *array_copy = NULL;

if (flags != KAS_BORROWS_ARRAY && flags != 0) {
ret = KAS_ERR_BAD_FLAGS;
goto out;
}

if (type < 0 || type >= KAS_NUM_TYPES) {
ret = KAS_ERR_BAD_TYPE;
goto out;
}
if (flags & KAS_BORROWS_ARRAY) {
ret = kastore_bput(self, key, key_len, array, array_len, type, flags);
} else {
array_size = type_size(type) * array_len;
array_copy = malloc(array_size == 0 ? 1 : array_size);
if (array_copy == NULL) {
ret = KAS_ERR_NO_MEMORY;
goto out;
}
memcpy(array_copy, array, array_size);
ret = kastore_oput(self, key, key_len, array_copy, array_len, type, flags);
if (ret == 0) {
/* Kastore has taken ownership of the array, so we don't need to free it */
array_copy = NULL;
}
}
out:
kas_safe_free(array_copy);
return ret;
}

int KAS_WARN_UNUSED
kastore_oput(kastore_t *self, const char *key, size_t key_len, void *array,
size_t array_len, int type, int flags)
{
int ret = 0;
kaitem_t *item;

if (flags != 0) {
ret = KAS_ERR_BAD_FLAGS;
goto out;
}

ret = kastore_put_item(self, &item, key, key_len, type, flags);
if (ret != 0) {
goto out;
}
item->borrowed_array = array;
item->array = array;
item->array_len = array_len;
out:
return ret;
Expand Down
25 changes: 13 additions & 12 deletions c/kastore.h
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,9 @@ Unknown flags were provided to open.
/* Flags for open */
#define KAS_READ_ALL (1 << 0)
#define KAS_GET_TAKES_OWNERSHIP (1 << 1)
#define KAS_PUT_BORROWS_ARRAY (1 << 2)

/* Flags for put */
#define KAS_BORROWS_ARRAY (1 << 8)


/**
Expand Down Expand Up @@ -191,7 +193,7 @@ typedef struct {
size_t key_len;
size_t array_len;
char *key;
/* Used when KAS_PUT_BORROWS_ARRAY is set */
/* Used when KAS_BORROWS_ARRAY is set */
const void *borrowed_array;
void *array;
size_t key_start;
Expand Down Expand Up @@ -430,12 +432,14 @@ int kastore_gets_float64(
@rst
A key with the specified length is inserted into the store and associated with
an array of the specified type and number of elements. The contents of the
specified key and array are copied. Keys can be any sequence of bytes but must
be at least one byte long and be unique. There is no restriction on the
contents of arrays. This is the most general form of ``put`` operation in
kastore; when the type of the array is known and the keys are standard C
strings, it is usually more convenient to use the :ref:`typed variants
<sec_c_api_typed_put>` of this function.
specified key and array are copied unless the KAS_BORROWS_ARRAY flag is specified.
If KAS_BORROWS_ARRAY is specified the array buffer must persist until the
kastore is closed.
Keys can be any sequence of bytes but must be at least one byte long and be
unique. There is no restriction on the contents of arrays. This is the most
general form of ``put`` operation in kastore; when the type of the array
is known and the keys are standard C strings, it is usually more convenient
to use the :ref:`typed variants <sec_c_api_typed_put>` of this function.
@endrst
@param self A pointer to a kastore object.
Expand All @@ -444,7 +448,7 @@ strings, it is usually more convenient to use the :ref:`typed variants
@param array The array.
@param array_len The number of elements in the array.
@param type The type of the array.
@param flags The insertion flags. Currently unused.
@param flags The insertion flags, only KAS_BORROWS_ARRAY or 0 is a valid.
@return Return 0 on success or a negative value on failure.
*/
int kastore_put(kastore_t *self, const char *key, size_t key_len, const void *array,
Expand Down Expand Up @@ -495,9 +499,6 @@ int kastore_puts_float64(

/** @} */

int kastore_bput(kastore_t *self, const char *key, size_t key_len, const void *array,
size_t array_len, int type, int flags);

/**
@brief Insert the specified key-array pair into the store, transferring ownership
of the malloced array buffer to the store (own-put).
Expand Down
51 changes: 46 additions & 5 deletions c/tests.c
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ test_bad_open_flags(void)
{
int ret;
kastore_t store;
const int bad_flags[] = { 8, 9, 1 << 31, -1 };
const int bad_flags[] = { 4, 5, 1 << 31, -1 };
size_t j;

for (j = 0; j < sizeof(bad_flags) / sizeof(*bad_flags); j++) {
Expand Down Expand Up @@ -471,6 +471,46 @@ test_duplicate_key_oput(void)
ret = kastore_close(&store);
}

static void
test_bad_flag_put(void)
{
int ret;
kastore_t store;
uint32_t *a = calloc(1, sizeof(uint32_t));

ret = kastore_open(&store, _tmp_file_name, "w", 0);
CU_ASSERT_EQUAL_FATAL(ret, 0);

ret = kastore_put(&store, "a", 1, a, 1, KAS_UINT32, 0);
CU_ASSERT_EQUAL_FATAL(ret, 0);
ret = kastore_put(&store, "b", 1, a, 1, KAS_UINT32, 5);
CU_ASSERT_EQUAL_FATAL(ret, KAS_ERR_BAD_FLAGS);

ret = kastore_close(&store);
CU_ASSERT_EQUAL_FATAL(ret, 0);

kas_safe_free(a);
}

static void
test_bad_flag_oput(void)
{
int ret;
kastore_t store;
uint32_t *a = calloc(1, sizeof(uint32_t));

ret = kastore_open(&store, _tmp_file_name, "w", 0);
CU_ASSERT_EQUAL_FATAL(ret, 0);

ret = kastore_oput(&store, "a", 1, a, 1, KAS_UINT32, 0);
CU_ASSERT_EQUAL_FATAL(ret, 0);
ret = kastore_oput(&store, "b", 1, a, 1, KAS_UINT32, 1);
CU_ASSERT_EQUAL_FATAL(ret, KAS_ERR_BAD_FLAGS);

ret = kastore_close(&store);
CU_ASSERT_EQUAL_FATAL(ret, 0);
}

static void
test_empty_key(void)
{
Expand Down Expand Up @@ -740,14 +780,14 @@ test_borrow_array(void)
size_t array_len;
int type;

ret = kastore_open(&store, _tmp_file_name, "w", KAS_PUT_BORROWS_ARRAY);
ret = kastore_open(&store, _tmp_file_name, "w", 0);
CU_ASSERT_EQUAL_FATAL(ret, 0);

ret = kastore_puts(&store, "c", array, 4, KAS_UINT32, 0);
CU_ASSERT_EQUAL_FATAL(ret, 0);
ret = kastore_puts(&store, "b", array, 2, KAS_UINT32, 0);
ret = kastore_puts(&store, "b", array, 2, KAS_UINT32, KAS_BORROWS_ARRAY);
CU_ASSERT_EQUAL_FATAL(ret, 0);
ret = kastore_puts(&store, "a", array, 1, KAS_UINT32, 0);
ret = kastore_puts_uint32(&store, "a", array, 1, KAS_BORROWS_ARRAY);
CU_ASSERT_EQUAL_FATAL(ret, 0);

ret = kastore_close(&store);
Expand Down Expand Up @@ -905,7 +945,6 @@ test_simple_round_trip_oput_buffers(void)

ret = kastore_open(&store, _tmp_file_name, "w", 0);
CU_ASSERT_EQUAL_FATAL(ret, 0);

ret = kastore_oputs_uint32(&store, "c", array_c, 4, 0);
CU_ASSERT_EQUAL_FATAL(ret, 0);
ret = kastore_oputs_uint32(&store, "b", array_b, 2, 0);
Expand Down Expand Up @@ -1782,6 +1821,8 @@ main(int argc, char **argv)
{ "test_put_copy_array", test_put_copy_array },
{ "test_duplicate_key", test_duplicate_key },
{ "test_duplicate_key_oput", test_duplicate_key_oput },
{ "test_bad_flag_oput", test_bad_flag_oput },
{ "test_bad_flag_put", test_bad_flag_put },
{ "test_missing_key", test_missing_key },
{ "test_contains", test_contains },
{ "test_bad_types", test_bad_types },
Expand Down

0 comments on commit 6f95d00

Please sign in to comment.