Fix static analyzer warnings for adding empty data/strings (#170)
CBOR allows empty text and byte strings and QBOR supports them with AddText() and AddBytes() with a size of zero.
This results in a call to memmove with a NULL pointer. It was always with a zero length. Implementations of memmove tolerate this, but it is not recommended and static analyzers complain.
This PR fixes this. It also adds tests for this condition. It also documents that adding strings of zero length is supported in UsefulBuf.
This addresses #164
* Correct support for adding empty text/byte strings
* Update change log
Co-authored-by: Laurence Lundblade <lgl@securitytheory.com>
diff --git a/inc/qcbor/UsefulBuf.h b/inc/qcbor/UsefulBuf.h
index e1b3146..aa24507 100644
--- a/inc/qcbor/UsefulBuf.h
+++ b/inc/qcbor/UsefulBuf.h
@@ -42,6 +42,7 @@
when who what, where, why
-------- ---- --------------------------------------------------
+ 19/12/2022 llundblade Document that adding empty data is allowed.
4/11/2022 llundblade Add GetOutPlace and Advance to UsefulOutBuf.
9/21/2021 llundbla Clarify UsefulOutBuf size calculation mode
8/8/2021 dthaler/llundbla Work with C++ without compiler extensions
@@ -947,6 +948,8 @@
* Overlapping buffers are OK. @c NewData can point to data in the
* output buffer.
*
+ * NewData.len may be 0 in which case nothing will be inserted.
+ *
* If an error occurs, an error state is set in the @ref
* UsefulOutBuf. No error is returned. All subsequent attempts to add
* data will do nothing.
diff --git a/src/UsefulBuf.c b/src/UsefulBuf.c
index 1a6b385..b36e5d0 100644
--- a/src/UsefulBuf.c
+++ b/src/UsefulBuf.c
@@ -41,7 +41,8 @@
when who what, where, why
-------- ---- ---------------------------------------------------
- 4/11/2022 llundblade Add GetOutPlace and Advance to UsefulOutBuf
+ 19/12/2022 llundblade Don't pass NULL to memmove when adding empty data.
+ 4/11/2022 llundblade Add GetOutPlace and Advance to UsefulOutBuf
3/6/2021 mcr/llundblade Fix warnings related to --Wcast-qual
01/28/2020 llundblade Refine integer signedness to quiet static analysis.
01/08/2020 llundblade Documentation corrections & improved code formatting.
@@ -266,7 +267,9 @@
/* 4. Put the new data in */
uint8_t *pInsertionPoint = pSourceOfMove;
// To know memmove won't go off end of destination, see PtrMath #5
- memmove(pInsertionPoint, NewData.ptr, NewData.len);
+ if(NewData.ptr != NULL) {
+ memmove(pInsertionPoint, NewData.ptr, NewData.len);
+ }
}
pMe->data_len += NewData.len;
diff --git a/test/UsefulBuf_Tests.c b/test/UsefulBuf_Tests.c
index e729ff1..e93a011 100644
--- a/test/UsefulBuf_Tests.c
+++ b/test/UsefulBuf_Tests.c
@@ -57,6 +57,12 @@
/* add a space to end */
UsefulOutBuf_AppendByte(pUOB, ' ');
+ /* Add an empty string */
+ UsefulOutBuf_AppendUsefulBuf(pUOB, NULLUsefulBufC);
+
+ /* Add a zero length string (valid pointer, 0 length) */
+ UsefulOutBuf_AppendData(pUOB, "xxx", 0);
+
/* Add 6 bytes to the end */
UsefulBufC UBC = {"hunny ", 6};
UsefulOutBuf_AppendUsefulBuf(pUOB, UBC);
diff --git a/test/qcbor_encode_tests.c b/test/qcbor_encode_tests.c
index aca29ec..6e569ca 100644
--- a/test/qcbor_encode_tests.c
+++ b/test/qcbor_encode_tests.c
@@ -281,7 +281,7 @@
static const uint8_t spExpectedEncodedAll[] = {
- 0x98, 0x22, 0x66, 0x55, 0x49, 0x4e, 0x54, 0x36, 0x32, 0xd8,
+ 0x98, 0x23, 0x66, 0x55, 0x49, 0x4e, 0x54, 0x36, 0x32, 0xd8,
0x64, 0x1a, 0x05, 0x5d, 0x23, 0x15, 0x65, 0x49, 0x4e, 0x54,
0x36, 0x34, 0xd8, 0x4c, 0x1b, 0x00, 0x00, 0x00, 0x12, 0x16,
0xaf, 0x2b, 0x15, 0x00, 0x38, 0x2b, 0xa4, 0x63, 0x4c, 0x42,
@@ -296,12 +296,14 @@
0x69, 0x65, 0xc1, 0x1a, 0x53, 0x72, 0x4e, 0x00, 0x66, 0x74,
0x69, 0x6d, 0x65, 0x28, 0x29, 0xc1, 0x1a, 0x58, 0x0d, 0x41,
0x72, 0x39, 0x07, 0xb0, 0xc1, 0x1a, 0x58, 0x0d, 0x3f, 0x76,
- 0x42, 0xff, 0x00, 0xa3, 0x66, 0x62, 0x69, 0x6e, 0x62, 0x69,
- 0x6e, 0xda, 0x00, 0x01, 0x86, 0xa0, 0x41, 0x00, 0x66, 0x62,
+ 0x42, 0xff, 0x00, 0xa4, 0x66, 0x62, 0x69, 0x6e, 0x62, 0x69,
+ 0x6e, 0xda, 0x00, 0x01, 0x86, 0xa0, 0x41, 0x00,
+ 0x65, 0x65, 0x6D, 0x70, 0x74, 0x79, 0x40,
+ 0x66, 0x62,
0x6c, 0x61, 0x62, 0x65, 0x6c, 0x43, 0x01, 0x02, 0x03, 0x00,
0x44, 0x04, 0x02, 0x03, 0xfe, 0x6f, 0x62, 0x61, 0x72, 0x20,
0x62, 0x61, 0x72, 0x20, 0x66, 0x6f, 0x6f, 0x20, 0x62, 0x61,
- 0x72, 0x64, 0x6f, 0x6f, 0x66, 0x0a, 0xd8, 0x20, 0x78, 0x6b,
+ 0x72, 0x64, 0x6f, 0x6f, 0x66, 0x0a, 0x60, 0xd8, 0x20, 0x78, 0x6b,
0x68, 0x74, 0x74, 0x70, 0x3a, 0x2f, 0x2f, 0x73, 0x74, 0x61,
0x63, 0x6b, 0x6f, 0x76, 0x65, 0x72, 0x66, 0x6c, 0x6f, 0x77,
0x2e, 0x63, 0x6f, 0x6d, 0x2f, 0x71, 0x75, 0x65, 0x73, 0x74,
@@ -553,6 +555,7 @@
QCBOREncode_AddSZString(pECtx, "binbin");
QCBOREncode_AddTag(pECtx, 100000);
QCBOREncode_AddBytes(pECtx, ((UsefulBufC) {(uint8_t []){0x00}, 1}));
+ QCBOREncode_AddBytesToMap(pECtx, "empty", NULLUsefulBufC); // Empty string
QCBOREncode_AddBytesToMap(pECtx, "blabel", ((UsefulBufC) {(uint8_t []){0x01, 0x02, 0x03}, 3}));
QCBOREncode_AddBytesToMapN(pECtx, 0, ((UsefulBufC){(uint8_t []){0x04, 0x02, 0x03, 0xfe}, 4}));
QCBOREncode_CloseMap(pECtx);
@@ -560,6 +563,8 @@
/* text blobs */
QCBOREncode_AddText(pECtx, UsefulBuf_FROM_SZ_LITERAL("bar bar foo bar"));
QCBOREncode_AddSZString(pECtx, "oof\n");
+ QCBOREncode_AddText(pECtx, NULLUsefulBufC); // Empty string
+
const char *szURL =
"http://stackoverflow.com/questions/28059697/how-do-i-toggle-between-debug-and-release-builds-in-xcode-6-7-8";
QCBOREncode_AddURI(pECtx, UsefulBuf_FromSZ(szURL));