Fix for static analyzer warning in UsefulOutBuf
When used in size calculation mode (a null buffer pointer), UsefulOutBuf would do math with the null pointer. It never used the the computed pointer so there's no issue with the running code, but some static analyzers complain and it is technically not good C code so it is fixed.
----
This change combines the code under one NULL check for UsefulOutBuf.
This will allow not to do unnecessary work with nullptr arithmetic.
Also, this change removes the check for non-zero size for memmove,
because it can handle this situation
Found by PVS-Studio:
warning: V1004 [CWE-119] The '((nk_uint8_t *) pMe->UB.ptr)' pointer was used unsafely after it was verified against nullptr. Check lines: 261, 268.
diff --git a/src/UsefulBuf.c b/src/UsefulBuf.c
index f5149a5..1a6b385 100644
--- a/src/UsefulBuf.c
+++ b/src/UsefulBuf.c
@@ -254,22 +254,21 @@
}
/* 3. Slide existing data to the right */
- uint8_t *pSourceOfMove = ((uint8_t *)pMe->UB.ptr) + uInsertionPos; // PtrMath #1
- size_t uNumBytesToMove = pMe->data_len - uInsertionPos; // PtrMath #2
- uint8_t *pDestinationOfMove = pSourceOfMove + NewData.len; // PtrMath #3
+ if (!UsefulOutBuf_IsBufferNULL(pMe)) {
+ uint8_t *pSourceOfMove = ((uint8_t *)pMe->UB.ptr) + uInsertionPos; // PtrMath #1
+ size_t uNumBytesToMove = pMe->data_len - uInsertionPos; // PtrMath #2
+ uint8_t *pDestinationOfMove = pSourceOfMove + NewData.len; // PtrMath #3
- if(uNumBytesToMove && pMe->UB.ptr) {
// To know memmove won't go off end of destination, see PtrMath #4
// Use memove because it handles overlapping buffers
memmove(pDestinationOfMove, pSourceOfMove, uNumBytesToMove);
- }
- /* 4. Put the new data in */
- uint8_t *pInsertionPoint = ((uint8_t *)pMe->UB.ptr) + uInsertionPos; // PtrMath #5
- if(pMe->UB.ptr) {
- // To know memmove won't go off end of destination, see PtrMath #6
+ /* 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);
}
+
pMe->data_len += NewData.len;
}
@@ -295,9 +294,7 @@
Check #3 allows Check #2 to be refactored as NewData.Len > (me->size - uInsertionPos)
This algebraically rearranges to me->size > uInsertionPos + NewData.len
- PtrMath #5 is exactly the same as PtrMath #1
-
- PtrMath #6 will never wrap under because
+ PtrMath #5 will never wrap under because
Calculation for extent of memove is uRoomInDestination = me->UB.len - uInsertionPos;
Check #1 makes sure me->data_len is less than me->size
Check #3 makes sure uInsertionPos is less than me->data_len