make SetMemPool always have the same overhead; fixes to AllocAllStringsTest
diff --git a/inc/qcbor.h b/inc/qcbor.h
index 5d53f8d..c945757 100644
--- a/inc/qcbor.h
+++ b/inc/qcbor.h
@@ -632,8 +632,13 @@
/** During decoding, too many tags in the caller-configured tag list, or not
enough space in QCBORTagListOut. */
- QCBOR_ERR_TOO_MANY_TAGS
+ QCBOR_ERR_TOO_MANY_TAGS,
+ /** Returned by QCBORDecode_SetMemPool() when xx is too small. This should
+ never happen on a machine with 64-bit or smaller pointers. Fixing
+ it is probably by increasing QCBOR_DECODE_MIN_MEM_POOL_SIZE. */
+ QCBOR_ERR_MEM_POOL_INTERNAL
+
} QCBORError;
@@ -775,6 +780,16 @@
} QCBORStringAllocator;
+/**
+ This only matters if you use a string allocator
+ and and set it up with QCBORDecode_SetMemPool(). It is
+ the size of the overhead needed needed by
+ QCBORDecode_SetMemPool(). If you write your own
+ string allocator or use the separately available malloc
+ based string allocator, this size will not apply
+ */
+#define QCBOR_DECODE_MIN_MEM_POOL_SIZE 72
+
/**
This is used to tell the decoder about tags that it should
@@ -1637,42 +1652,40 @@
/**
- Set up the MemPool string allocator for indefinite length strings.
+ @brief Set up the MemPool string allocator for indefinite length strings.
@param[in] pCtx The decode context.
@param[in] MemPool The pointer and length of the memory pool.
@param[in] bAllStrings true means to put even definite length strings in the pool.
- @return 0 if the MemPool was at least minimum size, 1 if too small.
+ @return error if the MemPool was less than QCBOR_DECODE_MIN_MEM_POOL_SIZE.
- Indefinite length strings (text and byte) cannot be decoded unless there is
- a string allocator configured. MemPool is a simple built-in string allocator
- that allocates bytes from a memory pool handed to it by calling
- this function. The memory pool is just a pointer and length for some
- block of memory that is to be used for string allocation. It can
- come from the stack, heap or other.
+ Indefinite length strings (text and byte) cannot be decoded unless
+ there is a string allocator configured. MemPool is a simple built-in
+ string allocator that allocates bytes from a memory pool handed to it
+ by calling this function. The memory pool is just a pointer and
+ length for some block of memory that is to be used for string
+ allocation. It can come from the stack, heap or other.
- The memory pool must be large enough to hold some fixed overhead plus the
- space for all the strings allocated. The fixed overhead does vary
- by CPU and compiler, but can roughly be computed as the space for
- seven pointers, 56 bytes for a 64-bit CPU. There is no overhead
- per string allocated
+ The memory pool must be QCBOR_DECODE_MIN_MEM_POOL_SIZE plus space for
+ all the strings allocated. There is no overhead per string allocated
- This memory pool is used for all indefinite length strings that are text
- strings or byte strings, including strings used as labels.
+ This memory pool is used for all indefinite length strings that are
+ text strings or byte strings, including strings used as labels.
The pointers to strings in QCBORItem will point into the memory pool set
here. They do not need to be individually freed. Just discard the buffer
when they are no longer needed.
- If bAllStrings is set, then the size will be the overhead plus the space to
- hold **all** strings, definite and indefinite length, value or label. The
- advantage of this is that after the decode is complete, the original memory
- holding the encoded CBOR does not need to remain valid.
+ If bAllStrings is set, then the size will be the overhead plus the
+ space to hold **all** strings, definite and indefinite length, value
+ or label. The advantage of this is that after the decode is complete,
+ the original memory holding the encoded CBOR does not need to remain
+ valid.
- If this function is not called because there is no need to support indefinite
- length strings, the MemPool implementation should be dead-stripped by the loader
- and not add to code size.
+ If this function is never called because there is no need to support
+ indefinite length strings, the MemPool implementation should be
+ dead-stripped by the loader and not add to code size.
*/
QCBORError QCBORDecode_SetMemPool(QCBORDecodeContext *pCtx, UsefulBuf MemPool, bool bAllStrings);
diff --git a/src/qcbor_decode.c b/src/qcbor_decode.c
index b015dd8..8a1df27 100644
--- a/src/qcbor_decode.c
+++ b/src/qcbor_decode.c
@@ -1267,13 +1267,29 @@
*/
QCBORError QCBORDecode_SetMemPool(QCBORDecodeContext *me, UsefulBuf Pool, bool bAllStrings)
{
+ // The idea behind QCBOR_MIN_MEM_POOL_SIZE is
+ // that the caller knows exactly what size to
+ // allocate and that the tests can run conclusively
+ // no matter what size MemPool is
+ // even though it wastes some memory. MemPool
+ // will vary depending on pointer size of the
+ // the machine. QCBOR_MIN_MEM_POOL_SIZE is
+ // set for pointers up to 64-bits. This
+ // wastes about 50 bytes on a 32-bit machine.
+ // This check makes sure things don't go
+ // horribly wrong. It should optimize out
+ // when there is no problem as the sizes are
+ // known at compile time.
+ if(sizeof(MemPool) > QCBOR_DECODE_MIN_MEM_POOL_SIZE) {
+ return QCBOR_ERR_MEM_POOL_INTERNAL;
+ }
+
// The first bytes of the Pool passed in are used
// as the context (vtable of sorts) for the memory pool
// allocator.
- if(Pool.len < sizeof(MemPool)+1) {
+ if(Pool.len < QCBOR_DECODE_MIN_MEM_POOL_SIZE) {
return QCBOR_ERR_BUFFER_TOO_SMALL;
}
-
MemPool *pMP = (MemPool *)Pool.ptr;
// Fill in the "vtable"
@@ -1282,7 +1298,7 @@
pMP->StringAllocator.fDestructor = NULL;
// Set up the pointers to the memory to be allocated
- pMP->pStart = (uint8_t *)Pool.ptr + sizeof(MemPool);
+ pMP->pStart = (uint8_t *)Pool.ptr + QCBOR_DECODE_MIN_MEM_POOL_SIZE;
pMP->pFree = pMP->pStart;
pMP->pEnd = (uint8_t *)Pool.ptr + Pool.len;
diff --git a/test/qcbor_decode_tests.c b/test/qcbor_decode_tests.c
index be84e65..3cdec80 100644
--- a/test/qcbor_decode_tests.c
+++ b/test/qcbor_decode_tests.c
@@ -2416,7 +2416,7 @@
0x01 // integer being labeled.
};
-static UsefulBufC MakeIndefiniteBigBstr(UsefulBuf Storage)
+static UsefulBufC MakeIndefiniteBigBstr(UsefulBuf Storage) // TODO: size this
{
UsefulOutBuf UOB;
@@ -2458,7 +2458,7 @@
QCBORDecodeContext DC;
QCBORItem Item;
// big enough for MakeIndefiniteBigBstr() + MemPool overhead
- UsefulBuf_MAKE_STACK_UB(MemPool, 320);
+ UsefulBuf_MAKE_STACK_UB(MemPool, 350);
// --- Simple normal indefinite length string ------
UsefulBufC IndefLen = UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spIndefiniteLenString);
@@ -2637,32 +2637,37 @@
int AllocAllStringsTest()
{
QCBORDecodeContext DC;
+ QCBORError nCBORError;
+
// First test, use the "CSRMap" as easy input and checking
QCBORDecode_Init(&DC, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(spCSRInput), QCBOR_DECODE_MODE_NORMAL);
- UsefulBuf_MAKE_STACK_UB(Pool, 300);
+ UsefulBuf_MAKE_STACK_UB(Pool, sizeof(spCSRInput) + QCBOR_DECODE_MIN_MEM_POOL_SIZE);
- QCBORDecode_SetMemPool(&DC, Pool, 1); // Turn on copying.
-
- if(CheckCSRMaps(&DC)) {
+ nCBORError = QCBORDecode_SetMemPool(&DC, Pool, 1); // Turn on copying.
+ if(nCBORError) {
return -1;
}
+ if(CheckCSRMaps(&DC)) {
+ return -2;
+ }
+
// Next parse, save pointers to a few strings, destroy original and see all is OK.
- UsefulBuf_MAKE_STACK_UB(CopyOfStorage, 160);
+ UsefulBuf_MAKE_STACK_UB(CopyOfStorage, sizeof(pValidMapEncoded) + QCBOR_DECODE_MIN_MEM_POOL_SIZE);
const UsefulBufC CopyOf = UsefulBuf_Copy(CopyOfStorage, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(pValidMapEncoded));
QCBORDecode_Init(&DC, CopyOf, QCBOR_DECODE_MODE_NORMAL);
+ UsefulBuf_Set(Pool, '/');
QCBORDecode_SetMemPool(&DC, Pool, 1); // Turn on copying.
- int nCBORError;
QCBORItem Item1, Item2, Item3, Item4;
if((nCBORError = QCBORDecode_GetNext(&DC, &Item1)))
return nCBORError;
if(Item1.uDataType != QCBOR_TYPE_MAP ||
Item1.val.uCount != 3)
- return -1;
+ return -3;
if((nCBORError = QCBORDecode_GetNext(&DC, &Item1)))
return nCBORError;
if((nCBORError = QCBORDecode_GetNext(&DC, &Item2)))
@@ -2678,7 +2683,7 @@
Item1.uDataType != QCBOR_TYPE_INT64 ||
Item1.val.int64 != 42 ||
UsefulBuf_Compare(Item1.label.string, UsefulBuf_FromSZ("first integer"))) {
- return -1;
+ return -4;
}
@@ -2686,27 +2691,27 @@
UsefulBuf_Compare(Item2.label.string, UsefulBuf_FromSZ("an array of two strings")) ||
Item2.uDataType != QCBOR_TYPE_ARRAY ||
Item2.val.uCount != 2)
- return -1;
+ return -5;
if(Item3.uDataType != QCBOR_TYPE_TEXT_STRING ||
UsefulBuf_Compare(Item3.val.string, UsefulBuf_FromSZ("string1"))) {
- return -1;
+ return -6;
}
if(Item4.uDataType != QCBOR_TYPE_TEXT_STRING ||
UsefulBuf_Compare(Item4.val.string, UsefulBuf_FromSZ("string2"))) {
- return -1;
+ return -7;
}
// Next parse with a pool that is too small
- UsefulBuf_MAKE_STACK_UB(SmallPool, 80);
+ UsefulBuf_MAKE_STACK_UB(SmallPool, QCBOR_DECODE_MIN_MEM_POOL_SIZE + 1);
QCBORDecode_Init(&DC, UsefulBuf_FROM_BYTE_ARRAY_LITERAL(pValidMapEncoded), QCBOR_DECODE_MODE_NORMAL);
QCBORDecode_SetMemPool(&DC, SmallPool, 1); // Turn on copying.
if((nCBORError = QCBORDecode_GetNext(&DC, &Item1)))
- return nCBORError;
+ return -8;
if(Item1.uDataType != QCBOR_TYPE_MAP ||
Item1.val.uCount != 3) {
- return -1;
+ return -9;
}
if(!(nCBORError = QCBORDecode_GetNext(&DC, &Item1))){
if(!(nCBORError = QCBORDecode_GetNext(&DC, &Item2))) {
@@ -2716,7 +2721,7 @@
}
}
if(nCBORError != QCBOR_ERR_STRING_ALLOCATE) {
- return -5;
+ return -10;
}
return 0;
@@ -2735,7 +2740,10 @@
// Set up an memory pool of 100 bytes
UsefulBuf_MAKE_STACK_UB(Pool, 100);
- QCBORDecode_SetMemPool(&DC, Pool, 0);
+ QCBORError nError = QCBORDecode_SetMemPool(&DC, Pool, 0);
+ if(nError) {
+ return -9;
+ }
// Cheat a little to get to the string allocator object
// so we can call it directly to test it