Smart realloc for maps

PJK · May 9, 2015 · d001a34 · d001a34
1 parent 18ccbfd
commit d001a34
Show file tree

Hide file tree

Showing 5 changed files with 44 additions and 38 deletions.
diff --git a/src/cbor/arrays.c b/src/cbor/arrays.c
@@ -44,7 +44,6 @@ bool cbor_array_replace(cbor_item_t * item, size_t index, cbor_item_t * value)
 bool cbor_array_push(cbor_item_t *array, cbor_item_t *pushee)
 {
 	assert(cbor_isa_array(array));
-	cbor_incref(pushee);
 	struct _cbor_array_metadata *metadata = (struct _cbor_array_metadata *) &array->metadata;
 	cbor_item_t **data = (cbor_item_t **) array->data;
 	if (cbor_array_is_definite(array)) {
@@ -66,6 +65,7 @@ bool cbor_array_push(cbor_item_t *array, cbor_item_t *pushee)
 		}
 		((cbor_item_t **)array->data)[metadata->end_ptr++] = pushee;
 	}
+	cbor_incref(pushee);
 	return true;
 }
 

diff --git a/src/cbor/internal/builder_callbacks.c b/src/cbor/internal/builder_callbacks.c
@@ -46,10 +46,10 @@ void _cbor_builder_append(cbor_item_t *item, struct _cbor_decoder_context *ctx)
 			/* We use 0 and 1 subitems to distinguish between keys and values in indefinite items */
 			if (ctx->stack->top->subitems % 2) {
 				/* Odd record, this is a value */
-				cbor_map_add_value(ctx->stack->top->item, cbor_move(item));
+				_cbor_map_add_value(ctx->stack->top->item, cbor_move(item));
 			} else {
 				/* Even record, this is a key */
-				cbor_map_add_key(ctx->stack->top->item, cbor_move(item));
+				_cbor_map_add_key(ctx->stack->top->item, cbor_move(item));
 			}
 			if (cbor_map_is_definite(ctx->stack->top->item)) {
 				ctx->stack->top->subitems--;

diff --git a/src/cbor/maps.c b/src/cbor/maps.c
@@ -54,50 +54,56 @@ cbor_item_t *cbor_new_indefinite_map()
 	return item;
 }
 
-cbor_item_t *cbor_map_add(cbor_item_t *item, struct cbor_pair pair)
+
+bool _cbor_map_add_key(cbor_item_t *item, cbor_item_t *key)
 {
 	assert(cbor_isa_map(item));
-	cbor_incref(pair.value);
-	cbor_map_add_key(item, pair.key);
-	// TODO propagate realloc fail
-	cbor_map_handle(item)[
-		item->metadata.map_metadata.end_ptr - 1
-	].value = pair.value;
-	return item;
+	struct _cbor_map_metadata *metadata = (struct _cbor_map_metadata *) &item->metadata;
+	if (cbor_map_is_definite(item)) {
+		struct cbor_pair *data = cbor_map_handle(item);
+		if (metadata->end_ptr >= metadata->allocated)
+			/* Don't realloc definite preallocated map */
+			return false;
+		data[metadata->end_ptr].key = key;
+		data[metadata->end_ptr++].value = NULL;
+	} else {
+		if (metadata->end_ptr >= metadata->allocated) {
+			/* Exponential realloc */
+			size_t new_allocation = (size_t)(CBOR_BUFFER_GROWTH * (metadata->allocated));
+			new_allocation = new_allocation ? new_allocation : 1;
+			unsigned char * new_data = _CBOR_REALLOC(item->data,
+													 new_allocation * sizeof(struct cbor_pair));
+			if (new_data == NULL)
+				return false;
+			item->data = new_data;
+			metadata->allocated = new_allocation;
+		}
+		struct cbor_pair *data = cbor_map_handle(item);
+		data[metadata->end_ptr].key = key;
+		data[metadata->end_ptr++].value = NULL;
+	}
+	cbor_incref(key);
+	return true;
 }
 
-cbor_item_t *cbor_map_add_value(cbor_item_t *item, cbor_item_t *value)
+bool _cbor_map_add_value(cbor_item_t *item, cbor_item_t *value)
 {
 	assert(cbor_isa_map(item));
 	cbor_incref(value);
-	// TODO propagate realloc fail
 	cbor_map_handle(item)[
+		/* Move one back since we are assuming _add_key (which increased the ptr)
+		 * was the previous operation on this object */
 		item->metadata.map_metadata.end_ptr - 1
 	].value = value;
-	return item;
+	return true;
 }
 
-cbor_item_t *cbor_map_add_key(cbor_item_t *item, cbor_item_t *key)
+bool cbor_map_add(cbor_item_t *item, struct cbor_pair pair)
 {
 	assert(cbor_isa_map(item));
-	cbor_incref(key);
-	struct _cbor_map_metadata *metadata = (struct _cbor_map_metadata *) &item->metadata;
-	struct cbor_pair *data = cbor_map_handle(item);
-	if (cbor_map_is_definite(item)) {
-		if (metadata->end_ptr > metadata->allocated)
-			printf("Error - NE space def\n");
-		data[metadata->end_ptr].key = key;
-		data[metadata->end_ptr++].value = NULL;
-	} else {
-		// TODO exponential reallocs?
-		if (metadata->end_ptr > metadata->allocated)
-			printf("Error - NE space indef\n");
-		data = _CBOR_REALLOC(data, (++metadata->allocated) * sizeof(struct cbor_pair));
-		data[metadata->end_ptr].key = key;
-		data[metadata->end_ptr++].value = NULL;
-		item->data = (unsigned char *) data;
-	}
-	return item;
+	if (!_cbor_map_add_key(item, pair.key))
+		return false;
+	return _cbor_map_add_value(item, pair.value);
 }
 
 bool cbor_map_is_definite(const cbor_item_t *item)

diff --git a/src/cbor/maps.h b/src/cbor/maps.h
@@ -26,11 +26,11 @@ cbor_item_t *cbor_new_definite_map(const size_t size);
 
 cbor_item_t *cbor_new_indefinite_map();
 
-cbor_item_t *cbor_map_add(cbor_item_t *item, struct cbor_pair pair);
+bool cbor_map_add(cbor_item_t *item, struct cbor_pair pair);
 
-cbor_item_t *cbor_map_add_key(cbor_item_t *item, cbor_item_t *key);
+bool _cbor_map_add_key(cbor_item_t *item, cbor_item_t *key);
 
-cbor_item_t *cbor_map_add_value(cbor_item_t *item, cbor_item_t *value);
+bool _cbor_map_add_value(cbor_item_t *item, cbor_item_t *value);
 
 bool cbor_map_is_definite(const cbor_item_t *item);
 

diff --git a/test/cbor_serialize_test.c b/test/cbor_serialize_test.c
@@ -212,8 +212,8 @@ static void test_serialize_indefinite_map(void **state)
 	cbor_item_t *one = cbor_build_uint8(1);
 	cbor_item_t *two = cbor_build_uint8(2);
 
-	cbor_map_add(item, (struct cbor_pair){ .key = one, .value = two });
-	cbor_map_add(item, (struct cbor_pair){ .key = two, .value = one });
+	assert_true(cbor_map_add(item, (struct cbor_pair){ .key = one, .value = two }));
+	assert_true(cbor_map_add(item, (struct cbor_pair){ .key = two, .value = one }));
 
 	assert_int_equal(6, cbor_serialize(item, buffer, 512));
 	assert_memory_equal(buffer, ((unsigned char[]) {0xBF, 0x01, 0x02, 0x02, 0x01, 0xFF}), 6);