Refine use of integer variables to quiet static analyzers and reduce object code size a little
Note that the no bugs of consequence were found by the static analyzer. The analyzer used was codesafe.cn
diff --git a/src/ieee754.c b/src/ieee754.c
index ea44e1d..ef0adef 100644
--- a/src/ieee754.c
+++ b/src/ieee754.c
@@ -19,7 +19,8 @@
the assumption that the optimizer will do a good job. The LLVM
optimizer, -Os, does seem to do the job and the resulting object code
is smaller from combining code for the many different cases (normal,
- subnormal, infinity, zero...) for the conversions.
+ subnormal, infinity, zero...) for the conversions. GCC is no where near
+ as good.
This code has really long lines and is much easier to read because of
them. Some coding guidelines prefer 80 column lines (can they not afford
@@ -27,7 +28,8 @@
columns.
Dead stripping is also really helpful to get code size down when
- floating-point encoding is not needed.
+ floating-point encoding is not needed. (If this is put in a library
+ and linking is against the library, then dead stripping is automatic).
This code works solely using shifts and masks and thus has no
dependency on any math libraries. It can even work if the CPU doesn't
@@ -47,6 +49,10 @@
- https://en.wikipedia.org/wiki/IEEE_754 and subordinate pages
- https://stackoverflow.com/questions/19800415/why-does-ieee-754-reserve-so-many-nan-values
+
+ - https://stackoverflow.com/questions/46073295/implicit-type-promotion-rules
+
+ - https://stackoverflow.com/questions/589575/what-does-the-c-standard-state-the-size-of-int-long-type-to-be
*/
@@ -97,10 +103,10 @@
0xfe 254 127 Largest normal exponent
0xff 255 128 NaN and Infinity */
#define SINGLE_EXPONENT_BIAS (127)
-#define SINGLE_EXPONENT_MAX (SINGLE_EXPONENT_BIAS) // 127 unbiased
+#define SINGLE_EXPONENT_MAX (SINGLE_EXPONENT_BIAS) // 127 unbiased
#define SINGLE_EXPONENT_MIN (-SINGLE_EXPONENT_BIAS+1) // -126 unbiased
#define SINGLE_EXPONENT_ZERO (-SINGLE_EXPONENT_BIAS) // -127 unbiased
-#define SINGLE_EXPONENT_INF_OR_NAN (SINGLE_EXPONENT_BIAS+1) // 128 unbiased
+#define SINGLE_EXPONENT_INF_OR_NAN (SINGLE_EXPONENT_BIAS+1) // 128 unbiased
// --------- Double-Precision ----------
@@ -132,14 +138,13 @@
/*
- Convenient functions to avoid type punning, compiler warnings and such
- The optimizer reduces them to a simple assignment.
- This is a crusty corner of C. It shouldn't be this hard.
+ Convenient functions to avoid type punning, compiler warnings and
+ such. The optimizer reduces them to a simple assignment. This is a
+ crusty corner of C. It shouldn't be this hard.
These are also in UsefulBuf.h under a different name. They are copied
- here to avoid a dependency on UsefulBuf.h. There is no
- object code size impact because these always optimze down to a
- simple assignment.
+ here to avoid a dependency on UsefulBuf.h. There is no object code
+ size impact because these always optimze down to a simple assignment.
*/
static inline uint32_t CopyFloatToUint32(float f)
{
@@ -175,13 +180,18 @@
{
// Pull the three parts out of the single-precision float
const uint32_t uSingle = CopyFloatToUint32(f);
- const int32_t nSingleUnbiasedExponent = ((uSingle & SINGLE_EXPONENT_MASK) >> SINGLE_EXPONENT_SHIFT) - SINGLE_EXPONENT_BIAS;
- const uint32_t uSingleSign = (uSingle & SINGLE_SIGN_MASK) >> SINGLE_SIGN_SHIFT;
- const uint32_t uSingleSignificand = uSingle & SINGLE_SIGNIFICAND_MASK;
+ const int32_t nSingleUnbiasedExponent = (int32_t)((uSingle & SINGLE_EXPONENT_MASK) >> SINGLE_EXPONENT_SHIFT) - SINGLE_EXPONENT_BIAS;
+ const uint32_t uSingleSign = (uSingle & SINGLE_SIGN_MASK) >> SINGLE_SIGN_SHIFT;
+ const uint32_t uSingleSignificand = uSingle & SINGLE_SIGNIFICAND_MASK;
// Now convert the three parts to half-precision.
- uint16_t uHalfSign, uHalfSignificand, uHalfBiasedExponent;
+
+ // All works is done on uint32_t with conversion to uint16_t at the end.
+ // This avoids integer promotions that static analyzers complain about and
+ // reduces code size.
+ uint32_t uHalfSign, uHalfSignificand, uHalfBiasedExponent;
+
if(nSingleUnbiasedExponent == SINGLE_EXPONENT_INF_OR_NAN) {
// +/- Infinity and NaNs -- single biased exponent is 0xff
uHalfBiasedExponent = HALF_EXPONENT_INF_OR_NAN + HALF_EXPONENT_BIAS;
@@ -189,13 +199,13 @@
// Infinity
uHalfSignificand = 0;
} else {
- // Copy the LBSs of the NaN payload that will fit from the single to the half
+ // Copy the LSBs of the NaN payload that will fit from the single to the half
uHalfSignificand = uSingleSignificand & (HALF_SIGNIFICAND_MASK & ~HALF_QUIET_NAN_BIT);
if(uSingleSignificand & SINGLE_QUIET_NAN_BIT) {
// It's a qNaN; copy the qNaN bit
uHalfSignificand |= HALF_QUIET_NAN_BIT;
} else {
- // It's a sNaN; make sure the significand is not zero so it stays a NaN
+ // It's an sNaN; make sure the significand is not zero so it stays a NaN
// This is needed because not all significand bits are copied from single
if(!uHalfSignificand) {
// Set the LSB. This is what wikipedia shows for sNAN.
@@ -213,26 +223,28 @@
uHalfSignificand = 0;
} else if(nSingleUnbiasedExponent < HALF_EXPONENT_MIN) {
// Exponent is too small to express in half-precision normal; make it a half-precision subnormal
- uHalfBiasedExponent = (uint16_t)(HALF_EXPONENT_ZERO + HALF_EXPONENT_BIAS);
+ uHalfBiasedExponent = HALF_EXPONENT_ZERO + HALF_EXPONENT_BIAS;
// Difference between single normal exponent and the base exponent of a half subnormal
- const uint32_t nExpDiff = -(nSingleUnbiasedExponent - HALF_EXPONENT_MIN);
+ const uint32_t uExpDiff = (uint32_t)-(nSingleUnbiasedExponent - HALF_EXPONENT_MIN);
// Also have to shift the significand by the difference in number of bits between a single and a half significand
- const int32_t nSignificandBitsDiff = SINGLE_NUM_SIGNIFICAND_BITS - HALF_NUM_SIGNIFICAND_BITS;
+ const uint32_t uSignificandBitsDiff = SINGLE_NUM_SIGNIFICAND_BITS - HALF_NUM_SIGNIFICAND_BITS;
// Add in the 1 that is implied in the significand of a normal number; it needs to be present in a subnormal
- const uint32_t uSingleSignificandSubnormal = uSingleSignificand + (0x01L << SINGLE_NUM_SIGNIFICAND_BITS);
- uHalfSignificand = uSingleSignificandSubnormal >> (nExpDiff + nSignificandBitsDiff);
+ const uint32_t uSingleSignificandSubnormal = uSingleSignificand + (0x01UL << SINGLE_NUM_SIGNIFICAND_BITS);
+ uHalfSignificand = uSingleSignificandSubnormal >> (uExpDiff + uSignificandBitsDiff);
} else {
- // The normal case
- uHalfBiasedExponent = nSingleUnbiasedExponent + HALF_EXPONENT_BIAS;
+ // The normal case, exponent is in range for half-precision
+ uHalfBiasedExponent = (uint32_t)(nSingleUnbiasedExponent + HALF_EXPONENT_BIAS);
uHalfSignificand = uSingleSignificand >> (SINGLE_NUM_SIGNIFICAND_BITS - HALF_NUM_SIGNIFICAND_BITS);
}
uHalfSign = uSingleSign;
// Put the 3 values in the right place for a half precision
- const uint16_t uHalfPrecision = uHalfSignificand |
+ const uint32_t uHalfPrecision = uHalfSignificand |
(uHalfBiasedExponent << HALF_EXPONENT_SHIFT) |
(uHalfSign << HALF_SIGN_SHIFT);
- return uHalfPrecision;
+ // Cast is safe because all the masks and shifts above work to make
+ // a half precision value which is only 16 bits.
+ return (uint16_t)uHalfPrecision;
}
@@ -241,13 +253,19 @@
{
// Pull the three parts out of the double-precision float
const uint64_t uDouble = CopyDoubleToUint64(d);
- const int64_t nDoubleUnbiasedExponent = ((uDouble & DOUBLE_EXPONENT_MASK) >> DOUBLE_EXPONENT_SHIFT) - DOUBLE_EXPONENT_BIAS;
- const uint64_t uDoubleSign = (uDouble & DOUBLE_SIGN_MASK) >> DOUBLE_SIGN_SHIFT;
- const uint64_t uDoubleSignificand = uDouble & DOUBLE_SIGNIFICAND_MASK;
-
+ const int64_t nDoubleUnbiasedExponent = (int64_t)((uDouble & DOUBLE_EXPONENT_MASK) >> DOUBLE_EXPONENT_SHIFT) - DOUBLE_EXPONENT_BIAS;
+ const uint64_t uDoubleSign = (uDouble & DOUBLE_SIGN_MASK) >> DOUBLE_SIGN_SHIFT;
+ const uint64_t uDoubleSignificand = uDouble & DOUBLE_SIGNIFICAND_MASK;
// Now convert the three parts to half-precision.
- uint16_t uHalfSign, uHalfSignificand, uHalfBiasedExponent;
+
+ // All works is done on uint64_t with conversion to uint16_t at the end.
+ // This avoids integer promotions that static analyzers complain about.
+ // Other options are for these to be unsigned int or fast_int16_t. Code
+ // size doesn't vary much between all these options for 64-bit LLVM,
+ // 64-bit GCC and 32-bit Armv7 LLVM.
+ uint64_t uHalfSign, uHalfSignificand, uHalfBiasedExponent;
+
if(nDoubleUnbiasedExponent == DOUBLE_EXPONENT_INF_OR_NAN) {
// +/- Infinity and NaNs -- single biased exponent is 0xff
uHalfBiasedExponent = HALF_EXPONENT_INF_OR_NAN + HALF_EXPONENT_BIAS;
@@ -255,7 +273,7 @@
// Infinity
uHalfSignificand = 0;
} else {
- // Copy the LBSs of the NaN payload that will fit from the double to the half
+ // Copy the LSBs of the NaN payload that will fit from the double to the half
uHalfSignificand = uDoubleSignificand & (HALF_SIGNIFICAND_MASK & ~HALF_QUIET_NAN_BIT);
if(uDoubleSignificand & DOUBLE_QUIET_NAN_BIT) {
// It's a qNaN; copy the qNaN bit
@@ -279,37 +297,42 @@
uHalfSignificand = 0;
} else if(nDoubleUnbiasedExponent < HALF_EXPONENT_MIN) {
// Exponent is too small to express in half-precision; round down to zero
- uHalfBiasedExponent = (uint16_t)(HALF_EXPONENT_ZERO + HALF_EXPONENT_BIAS);
+ uHalfBiasedExponent = HALF_EXPONENT_ZERO + HALF_EXPONENT_BIAS;
// Difference between double normal exponent and the base exponent of a half subnormal
- const uint64_t nExpDiff = -(nDoubleUnbiasedExponent - HALF_EXPONENT_MIN);
+ const uint64_t uExpDiff = (uint64_t)-(nDoubleUnbiasedExponent - HALF_EXPONENT_MIN);
// Also have to shift the significand by the difference in number of bits between a double and a half significand
- const int64_t nSignificandBitsDiff = DOUBLE_NUM_SIGNIFICAND_BITS - HALF_NUM_SIGNIFICAND_BITS;
+ const uint64_t uSignificandBitsDiff = DOUBLE_NUM_SIGNIFICAND_BITS - HALF_NUM_SIGNIFICAND_BITS;
// Add in the 1 that is implied in the significand of a normal number; it needs to be present in a subnormal
const uint64_t uDoubleSignificandSubnormal = uDoubleSignificand + (0x01ULL << DOUBLE_NUM_SIGNIFICAND_BITS);
- uHalfSignificand = uDoubleSignificandSubnormal >> (nExpDiff + nSignificandBitsDiff);
+ uHalfSignificand = uDoubleSignificandSubnormal >> (uExpDiff + uSignificandBitsDiff);
} else {
- // The normal case
- uHalfBiasedExponent = nDoubleUnbiasedExponent + HALF_EXPONENT_BIAS;
+ // The normal case, exponent is in range for half-precision
+ uHalfBiasedExponent = (uint32_t)(nDoubleUnbiasedExponent + HALF_EXPONENT_BIAS);
uHalfSignificand = uDoubleSignificand >> (DOUBLE_NUM_SIGNIFICAND_BITS - HALF_NUM_SIGNIFICAND_BITS);
}
uHalfSign = uDoubleSign;
// Put the 3 values in the right place for a half precision
- const uint16_t uHalfPrecision = uHalfSignificand |
+ const uint64_t uHalfPrecision = uHalfSignificand |
(uHalfBiasedExponent << HALF_EXPONENT_SHIFT) |
(uHalfSign << HALF_SIGN_SHIFT);
- return uHalfPrecision;
+ // Cast is safe because all the masks and shifts above work to make
+ // a half precision value which is only 16 bits.
+ return (uint16_t)uHalfPrecision;
}
+
// Public function; see ieee754.h
float IEEE754_HalfToFloat(uint16_t uHalfPrecision)
{
// Pull out the three parts of the half-precision float
- const uint16_t uHalfSignificand = uHalfPrecision & HALF_SIGNIFICAND_MASK;
- const int16_t nHalfUnBiasedExponent = ((uHalfPrecision & HALF_EXPONENT_MASK) >> HALF_EXPONENT_SHIFT) - HALF_EXPONENT_BIAS;
- const uint16_t uHalfSign = (uHalfPrecision & HALF_SIGN_MASK) >> HALF_SIGN_SHIFT;
+ // Do all the work in 32 bits because that is what the end result is
+ // may give smaller code size and will keep static analyzers happier.
+ const uint32_t uHalfSignificand = uHalfPrecision & HALF_SIGNIFICAND_MASK;
+ const int32_t nHalfUnBiasedExponent = (int32_t)((uHalfPrecision & HALF_EXPONENT_MASK) >> HALF_EXPONENT_SHIFT) - HALF_EXPONENT_BIAS;
+ const uint32_t uHalfSign = (uHalfPrecision & HALF_SIGN_MASK) >> HALF_SIGN_SHIFT;
// Make the three parts of the single-precision number
@@ -350,12 +373,11 @@
}
} else {
// Normal number
- uSingleBiasedExponent = nHalfUnBiasedExponent + SINGLE_EXPONENT_BIAS;
+ uSingleBiasedExponent = (uint32_t)(nHalfUnBiasedExponent + SINGLE_EXPONENT_BIAS);
uSingleSignificand = uHalfSignificand << (SINGLE_NUM_SIGNIFICAND_BITS - HALF_NUM_SIGNIFICAND_BITS);
}
uSingleSign = uHalfSign;
-
// Shift the three parts of the single-precision into place
const uint32_t uSinglePrecision = uSingleSignificand |
(uSingleBiasedExponent << SINGLE_EXPONENT_SHIFT) |
@@ -369,9 +391,11 @@
double IEEE754_HalfToDouble(uint16_t uHalfPrecision)
{
// Pull out the three parts of the half-precision float
- const uint16_t uHalfSignificand = uHalfPrecision & HALF_SIGNIFICAND_MASK;
- const int16_t nHalfUnBiasedExponent = ((uHalfPrecision & HALF_EXPONENT_MASK) >> HALF_EXPONENT_SHIFT) - HALF_EXPONENT_BIAS;
- const uint16_t uHalfSign = (uHalfPrecision & HALF_SIGN_MASK) >> HALF_SIGN_SHIFT;
+ // Do all the work in 64 bits because that is what the end result is
+ // may give smaller code size and will keep static analyzers happier.
+ const uint64_t uHalfSignificand = uHalfPrecision & HALF_SIGNIFICAND_MASK;
+ const int64_t nHalfUnBiasedExponent = (int64_t)((uHalfPrecision & HALF_EXPONENT_MASK) >> HALF_EXPONENT_SHIFT) - HALF_EXPONENT_BIAS;
+ const uint64_t uHalfSign = (uHalfPrecision & HALF_SIGN_MASK) >> HALF_SIGN_SHIFT;
// Make the three parts of hte single-precision number
@@ -412,8 +436,8 @@
}
} else {
// Normal number
- uDoubleBiasedExponent = nHalfUnBiasedExponent + DOUBLE_EXPONENT_BIAS;
- uDoubleSignificand = (uint64_t)uHalfSignificand << (DOUBLE_NUM_SIGNIFICAND_BITS - HALF_NUM_SIGNIFICAND_BITS);
+ uDoubleBiasedExponent = (uint64_t)(nHalfUnBiasedExponent + DOUBLE_EXPONENT_BIAS);
+ uDoubleSignificand = uHalfSignificand << (DOUBLE_NUM_SIGNIFICAND_BITS - HALF_NUM_SIGNIFICAND_BITS);
}
uDoubleSign = uHalfSign;
@@ -433,7 +457,7 @@
// Pull the neeed two parts out of the single-precision float
const uint32_t uSingle = CopyFloatToUint32(f);
- const int32_t nSingleExponent = ((uSingle & SINGLE_EXPONENT_MASK) >> SINGLE_EXPONENT_SHIFT) - SINGLE_EXPONENT_BIAS;
+ const int32_t nSingleExponent = (int32_t)((uSingle & SINGLE_EXPONENT_MASK) >> SINGLE_EXPONENT_SHIFT) - SINGLE_EXPONENT_BIAS;
const uint32_t uSingleSignificand = uSingle & SINGLE_SIGNIFICAND_MASK;
// Bit mask that is the significand bits that would be lost when converting
@@ -469,7 +493,7 @@
// Pull the needed two parts out of the double-precision float
const uint64_t uDouble = CopyDoubleToUint64(d);
- const int64_t nDoubleExponent = ((uDouble & DOUBLE_EXPONENT_MASK) >> DOUBLE_EXPONENT_SHIFT) - DOUBLE_EXPONENT_BIAS;
+ const int64_t nDoubleExponent = (int64_t)((uDouble & DOUBLE_EXPONENT_MASK) >> DOUBLE_EXPONENT_SHIFT) - DOUBLE_EXPONENT_BIAS;
const uint64_t uDoubleSignificand = uDouble & DOUBLE_SIGNIFICAND_MASK;
// Masks to check whether dropped significand bits are zero or not
diff --git a/src/ieee754.h b/src/ieee754.h
index d2c1224..705ef62 100644
--- a/src/ieee754.h
+++ b/src/ieee754.h
@@ -24,7 +24,7 @@
+/- infinity, +/- zero, subnormal numbers, qNaN, sNaN and NaN
payloads.
- This confirms to IEEE 754-2008, but note that this doesn't specify
+ This conforms to IEEE 754-2008, but note that this doesn't specify
conversions, just the encodings.
NaN payloads are preserved with alignment on the LSB. The qNaN bit is
diff --git a/src/qcbor_decode.c b/src/qcbor_decode.c
index 26bc3f6..661264e 100644
--- a/src/qcbor_decode.c
+++ b/src/qcbor_decode.c
@@ -42,7 +42,9 @@
when who what, where, why
-------- ---- ---------------------------------------------------
- 01/08/2020 llundblade Documentation corrections & improved code formatting.
+ 01/25/2020 llundblade Cleaner handling of too-long encoded string input.
+ 01/25/2020 llundblade Refine use of integer types to quiet static analysis
+ 01/08/2020 llundblade Documentation corrections & improved code formatting
12/30/19 llundblade Add support for decimal fractions and bigfloats.
11/07/19 llundblade Fix long long conversion to double compiler warning
09/07/19 llundblade Fix bug decoding empty arrays and maps
@@ -107,7 +109,9 @@
inline static uint8_t
DecodeNesting_GetLevel(QCBORDecodeNesting *pNesting)
{
- return pNesting->pCurrent - &(pNesting->pMapsAndArrays[0]);
+ // Check in DecodeNesting_Descend and never having
+ // QCBOR_MAX_ARRAY_NESTING > 255 gaurantee cast is safe
+ return (uint8_t)(pNesting->pCurrent - &(pNesting->pMapsAndArrays[0]));
}
inline static int
@@ -440,49 +444,52 @@
This returns:
pnMajorType -- the major type for the item
- puNumber -- the "number" which is used a the value for integers,
+ puArgument -- the "number" which is used a the value for integers,
tags and floats and length for strings and arrays
- puAdditionalInfo -- Pass this along to know what kind of float or
+ pnAdditionalInfo -- Pass this along to know what kind of float or
if length is indefinite
+ The int type is preferred to uint8_t for some variables as this
+ avoids integer promotions, can reduce code size and makes
+ static analyzers happier.
*/
inline static QCBORError DecodeTypeAndNumber(UsefulInputBuf *pUInBuf,
int *pnMajorType,
uint64_t *puArgument,
- uint8_t *puAdditionalInfo)
+ int *pnAdditionalInfo)
{
QCBORError nReturn;
// Get the initial byte that every CBOR data item has
- const uint8_t uInitialByte = UsefulInputBuf_GetByte(pUInBuf);
+ const int nInitialByte = (int)UsefulInputBuf_GetByte(pUInBuf);
// Break down the initial byte
- const uint8_t uTmpMajorType = uInitialByte >> 5;
- const uint8_t uAdditionalInfo = uInitialByte & 0x1f;
+ const int nTmpMajorType = nInitialByte >> 5;
+ const int nAdditionalInfo = nInitialByte & 0x1f;
// Where the number or argument accumulates
uint64_t uArgument;
- if(uAdditionalInfo >= LEN_IS_ONE_BYTE && uAdditionalInfo <= LEN_IS_EIGHT_BYTES) {
+ if(nAdditionalInfo >= LEN_IS_ONE_BYTE && nAdditionalInfo <= LEN_IS_EIGHT_BYTES) {
// Need to get 1,2,4 or 8 additional argument bytes Map
// LEN_IS_ONE_BYTE.. LEN_IS_EIGHT_BYTES to actual length
static const uint8_t aIterate[] = {1,2,4,8};
// Loop getting all the bytes in the argument
uArgument = 0;
- for(int i = aIterate[uAdditionalInfo - LEN_IS_ONE_BYTE]; i; i--) {
+ for(int i = aIterate[nAdditionalInfo - LEN_IS_ONE_BYTE]; i; i--) {
// This shift and add gives the endian conversion
uArgument = (uArgument << 8) + UsefulInputBuf_GetByte(pUInBuf);
}
- } else if(uAdditionalInfo >= ADDINFO_RESERVED1 && uAdditionalInfo <= ADDINFO_RESERVED3) {
+ } else if(nAdditionalInfo >= ADDINFO_RESERVED1 && nAdditionalInfo <= ADDINFO_RESERVED3) {
// The reserved and thus-far unused additional info values
nReturn = QCBOR_ERR_UNSUPPORTED;
goto Done;
} else {
// Less than 24, additional info is argument or 31, an indefinite length
// No more bytes to get
- uArgument = uAdditionalInfo;
+ uArgument = (uint64_t)nAdditionalInfo;
}
if(UsefulInputBuf_GetError(pUInBuf)) {
@@ -492,14 +499,15 @@
// All successful if we got here.
nReturn = QCBOR_SUCCESS;
- *pnMajorType = uTmpMajorType;
+ *pnMajorType = nTmpMajorType;
*puArgument = uArgument;
- *puAdditionalInfo = uAdditionalInfo;
+ *pnAdditionalInfo = nAdditionalInfo;
Done:
return nReturn;
}
+
/*
CBOR doesn't explicitly specify two's compliment for integers but all
CPUs use it these days and the test vectors in the RFC are so. All
@@ -511,12 +519,16 @@
compliment to represent negative integers.
See http://www.unix.org/whitepapers/64bit.html for reasons int isn't
- used here in any way including in the interface
+ used carefully here, and in particular why it isn't used in the interface.
+ Also see
+ https://stackoverflow.com/questions/17489857/why-is-int-typically-32-bit-on-64-bit-compilers
+
+ Int is used for values that need less than 16-bits and would be subject
+ to integer promotion and complaining by static analyzers.
*/
inline static QCBORError
DecodeInteger(int nMajorType, uint64_t uNumber, QCBORItem *pDecodedItem)
{
- // Stack usage: int/ptr 1 -- 8
QCBORError nReturn = QCBOR_SUCCESS;
if(nMajorType == CBOR_MAJOR_TYPE_POSITIVE_INT) {
@@ -531,7 +543,11 @@
}
} else {
if(uNumber <= INT64_MAX) {
- pDecodedItem->val.int64 = -uNumber-1;
+ // CBOR's representation of negative numbers lines up with the
+ // two-compliment representation. A negative integer has one
+ // more in range than a positive integer. INT64_MIN is
+ // equal to (-INT64_MAX) - 1.
+ pDecodedItem->val.int64 = (-(int64_t)uNumber) - 1;
pDecodedItem->uDataType = QCBOR_TYPE_INT64;
} else {
@@ -577,16 +593,16 @@
Decode true, false, floats, break...
*/
inline static QCBORError
-DecodeSimple(uint8_t uAdditionalInfo, uint64_t uNumber, QCBORItem *pDecodedItem)
+DecodeSimple(int nAdditionalInfo, uint64_t uNumber, QCBORItem *pDecodedItem)
{
- // Stack usage: 0
QCBORError nReturn = QCBOR_SUCCESS;
// uAdditionalInfo is 5 bits from the initial byte compile time checks
- // above make sure uAdditionalInfo values line up with uDataType values
- pDecodedItem->uDataType = uAdditionalInfo;
+ // above make sure uAdditionalInfo values line up with uDataType values.
+ // DecodeTypeAndNumber never returns a major type > 1f so cast is safe
+ pDecodedItem->uDataType = (uint8_t)nAdditionalInfo;
- switch(uAdditionalInfo) {
+ switch(nAdditionalInfo) {
// No check for ADDINFO_RESERVED1 - ADDINFO_RESERVED3 as they are
// caught before this is called.
@@ -636,7 +652,6 @@
}
-
/*
Decode text and byte strings. Call the string allocator if asked to.
*/
@@ -646,10 +661,20 @@
UsefulInputBuf *pUInBuf,
QCBORItem *pDecodedItem)
{
- // Stack usage: UsefulBuf 2, int/ptr 1 40
QCBORError nReturn = QCBOR_SUCCESS;
- const UsefulBufC Bytes = UsefulInputBuf_GetUsefulBuf(pUInBuf, uStrLen);
+ // CBOR lengths can be 64 bits, but size_t is not 64 bits on all CPUs.
+ // This check makes the casts to size_t below safe.
+
+ // 4 bytes less than the largest sizeof() so this can be tested by
+ // putting a SIZE_MAX length in the CBOR test input (no one will
+ // care the limit on strings is 4 bytes shorter).
+ if(uStrLen > SIZE_MAX-4) {
+ nReturn = QCBOR_ERR_STRING_TOO_LONG;
+ goto Done;
+ }
+
+ const UsefulBufC Bytes = UsefulInputBuf_GetUsefulBuf(pUInBuf, (size_t)uStrLen);
if(UsefulBuf_IsNULLC(Bytes)) {
// Failed to get the bytes for this string item
nReturn = QCBOR_ERR_HIT_END;
@@ -658,7 +683,7 @@
if(pAllocator) {
// We are asked to use string allocator to make a copy
- UsefulBuf NewMem = StringAllocator_Allocate(pAllocator, uStrLen);
+ UsefulBuf NewMem = StringAllocator_Allocate(pAllocator, (size_t)uStrLen);
if(UsefulBuf_IsNULL(NewMem)) {
nReturn = QCBOR_ERR_STRING_ALLOCATE;
goto Done;
@@ -670,8 +695,9 @@
pDecodedItem->val.string = Bytes;
}
const bool bIsBstr = (nMajorType == CBOR_MAJOR_TYPE_BYTE_STRING);
- pDecodedItem->uDataType = bIsBstr ? QCBOR_TYPE_BYTE_STRING
- : QCBOR_TYPE_TEXT_STRING;
+ // Cast because ternary operator causes promotion to integer
+ pDecodedItem->uDataType = (uint8_t)(bIsBstr ? QCBOR_TYPE_BYTE_STRING
+ : QCBOR_TYPE_TEXT_STRING);
Done:
return nReturn;
@@ -705,7 +731,6 @@
QCBORItem *pDecodedItem,
const QCORInternalAllocator *pAllocator)
{
- // Stack usage: int/ptr 3 -- 24
QCBORError nReturn;
/*
@@ -714,13 +739,13 @@
an encoding of the length of the uNumber and is needed to decode
floats and doubles
*/
- int uMajorType;
+ int nMajorType;
uint64_t uNumber;
- uint8_t uAdditionalInfo;
+ int nAdditionalInfo;
memset(pDecodedItem, 0, sizeof(QCBORItem));
- nReturn = DecodeTypeAndNumber(pUInBuf, &uMajorType, &uNumber, &uAdditionalInfo);
+ nReturn = DecodeTypeAndNumber(pUInBuf, &nMajorType, &uNumber, &nAdditionalInfo);
// Error out here if we got into trouble on the type and number. The
// code after this will not work if the type and number is not good.
@@ -730,25 +755,25 @@
// At this point the major type and the value are valid. We've got
// the type and the number that starts every CBOR data item.
- switch (uMajorType) {
+ switch (nMajorType) {
case CBOR_MAJOR_TYPE_POSITIVE_INT: // Major type 0
case CBOR_MAJOR_TYPE_NEGATIVE_INT: // Major type 1
- if(uAdditionalInfo == LEN_IS_INDEFINITE) {
+ if(nAdditionalInfo == LEN_IS_INDEFINITE) {
nReturn = QCBOR_ERR_BAD_INT;
} else {
- nReturn = DecodeInteger(uMajorType, uNumber, pDecodedItem);
+ nReturn = DecodeInteger(nMajorType, uNumber, pDecodedItem);
}
break;
case CBOR_MAJOR_TYPE_BYTE_STRING: // Major type 2
case CBOR_MAJOR_TYPE_TEXT_STRING: // Major type 3
- if(uAdditionalInfo == LEN_IS_INDEFINITE) {
- const bool bIsBstr = (uMajorType == CBOR_MAJOR_TYPE_BYTE_STRING);
- pDecodedItem->uDataType = bIsBstr ? QCBOR_TYPE_BYTE_STRING
- : QCBOR_TYPE_TEXT_STRING;
+ if(nAdditionalInfo == LEN_IS_INDEFINITE) {
+ const bool bIsBstr = (nMajorType == CBOR_MAJOR_TYPE_BYTE_STRING);
+ pDecodedItem->uDataType = (uint8_t)(bIsBstr ? QCBOR_TYPE_BYTE_STRING
+ : QCBOR_TYPE_TEXT_STRING);
pDecodedItem->val.string = (UsefulBufC){NULL, SIZE_MAX};
} else {
- nReturn = DecodeBytes(pAllocator, uMajorType, uNumber, pUInBuf, pDecodedItem);
+ nReturn = DecodeBytes(pAllocator, nMajorType, uNumber, pUInBuf, pDecodedItem);
}
break;
@@ -759,18 +784,19 @@
nReturn = QCBOR_ERR_ARRAY_TOO_LONG;
goto Done;
}
- if(uAdditionalInfo == LEN_IS_INDEFINITE) {
+ if(nAdditionalInfo == LEN_IS_INDEFINITE) {
pDecodedItem->val.uCount = UINT16_MAX; // Indicate indefinite length
} else {
// type conversion OK because of check above
pDecodedItem->val.uCount = (uint16_t)uNumber;
}
// C preproc #if above makes sure constants for major types align
- pDecodedItem->uDataType = uMajorType;
+ // DecodeTypeAndNumber never returns a major type > 7 so cast is safe
+ pDecodedItem->uDataType = (uint8_t)nMajorType;
break;
case CBOR_MAJOR_TYPE_OPTIONAL: // Major type 6, optional prepended tags
- if(uAdditionalInfo == LEN_IS_INDEFINITE) {
+ if(nAdditionalInfo == LEN_IS_INDEFINITE) {
nReturn = QCBOR_ERR_BAD_INT;
} else {
pDecodedItem->val.uTagV = uNumber;
@@ -780,7 +806,7 @@
case CBOR_MAJOR_TYPE_SIMPLE:
// Major type 7, float, double, true, false, null...
- nReturn = DecodeSimple(uAdditionalInfo, uNumber, pDecodedItem);
+ nReturn = DecodeSimple(nAdditionalInfo, uNumber, pDecodedItem);
break;
default:
@@ -824,7 +850,8 @@
// indefinite length string tests, to be sure all is OK if this is removed.
// Only do indefinite length processing on strings
- if(pDecodedItem->uDataType != QCBOR_TYPE_BYTE_STRING && pDecodedItem->uDataType != QCBOR_TYPE_TEXT_STRING) {
+ if(pDecodedItem->uDataType != QCBOR_TYPE_BYTE_STRING &&
+ pDecodedItem->uDataType != QCBOR_TYPE_TEXT_STRING) {
goto Done; // no need to do any work here on non-string types
}
@@ -864,7 +891,8 @@
// Match data type of chunk to type at beginning.
// Also catches error of other non-string types that don't belong.
// Also catches indefinite length strings inside indefinite length strings
- if(StringChunkItem.uDataType != uStringType || StringChunkItem.val.string.len == SIZE_MAX) {
+ if(StringChunkItem.uDataType != uStringType ||
+ StringChunkItem.val.string.len == SIZE_MAX) {
nReturn = QCBOR_ERR_INDEFINITE_STRING_CHUNK;
break;
}
@@ -1161,7 +1189,8 @@
const UsefulBufC Temp = pDecodedItem->val.string;
pDecodedItem->val.bigNum = Temp;
const bool bIsPosBigNum = (bool)(pDecodedItem->uTagBits & QCBOR_TAGFLAG_POS_BIGNUM);
- pDecodedItem->uDataType = bIsPosBigNum ? QCBOR_TYPE_POSBIGNUM : QCBOR_TYPE_NEGBIGNUM;
+ pDecodedItem->uDataType = (uint8_t)(bIsPosBigNum ? QCBOR_TYPE_POSBIGNUM
+ : QCBOR_TYPE_NEGBIGNUM);
return QCBOR_SUCCESS;
}
diff --git a/src/qcbor_encode.c b/src/qcbor_encode.c
index cb6670d..ce14e41 100644
--- a/src/qcbor_encode.c
+++ b/src/qcbor_encode.c
@@ -42,6 +42,7 @@
when who what, where, why
-------- ---- ---------------------------------------------------
+ 01/25/2020 llundblade Refine use of integer types to quiet static analysis.
01/08/2020 llundblade Documentation corrections & improved code formatting.
12/30/19 llundblade Add support for decimal fractions and bigfloats.
8/7/19 llundblade Prevent encoding simple type reserved values 24..31
@@ -147,9 +148,13 @@
// items by two for maps to get the number of pairs. This implementation
// takes advantage of the map major type being one larger the array major
// type, hence uDivisor is either 1 or 2.
- const uint16_t uDivisor = pNesting->pCurrentNesting->uMajorType - CBOR_MAJOR_TYPE_ARRAY+1;
- return pNesting->pCurrentNesting->uCount / uDivisor;
+ if(pNesting->pCurrentNesting->uMajorType == CBOR_MAJOR_TYPE_MAP) {
+ // Cast back to uint16_t after integer promotion for bit shift
+ return (uint16_t)(pNesting->pCurrentNesting->uCount >> 1);
+ } else {
+ return pNesting->pCurrentNesting->uCount;
+ }
}
inline static uint32_t Nesting_GetStartPos(QCBORTrackNesting *pNesting)
@@ -329,6 +334,26 @@
Code Reviewers: THIS FUNCTION DOES POINTER MATH
*/
+ /*
+ The type int is used here for several variables because of the way
+ integer promotion works in C for integer variables that are
+ uint8_t or uint16_t. The basic rule is that they will always be
+ promoted to int if they will fit. All of these integer variables
+ need only hold values less than 255 or are promoted from uint8_t,
+ so they will always fit into an int. Note that promotion is only
+ to unsigned int if the value won't fit into an int even if the
+ promotion is for an unsigned like uint8_t.
+
+ By declaring them int, there are few implicit conversions and fewer
+ casts needed. Code size is reduced a little. It also makes static
+ analyzers happier.
+
+ Note also that declaring them uint8_t won't stop integer wrap
+ around if the code is wrong. It won't make the code more correct.
+
+ https://stackoverflow.com/questions/46073295/implicit-type-promotion-rules
+ https://stackoverflow.com/questions/589575/what-does-the-c-standard-state-the-size-of-int-long-type-to-be
+ */
// Holds up to 9 bytes of type and argument plus one extra so pointer
// always points to valid bytes.
@@ -336,20 +361,20 @@
// Point to the last bytes and work backwards
uint8_t *pByte = &bytes[sizeof(bytes)-1];
// This is the 5 bits in the initial byte that is not the major type
- uint8_t uAdditionalInfo;
+ int nAdditionalInfo;
if (uMajorType == CBOR_MAJOR_NONE_TYPE_ARRAY_INDEFINITE_LEN) {
uMajorType = CBOR_MAJOR_TYPE_ARRAY;
- uAdditionalInfo = LEN_IS_INDEFINITE;
+ nAdditionalInfo = LEN_IS_INDEFINITE;
} else if (uMajorType == CBOR_MAJOR_NONE_TYPE_MAP_INDEFINITE_LEN) {
uMajorType = CBOR_MAJOR_TYPE_MAP;
- uAdditionalInfo = LEN_IS_INDEFINITE;
+ nAdditionalInfo = LEN_IS_INDEFINITE;
} else if (uNumber < CBOR_TWENTY_FOUR && nMinLen == 0) {
// Simple case where argument is < 24
- uAdditionalInfo = uNumber;
+ nAdditionalInfo = (int)uNumber;
} else if (uMajorType == CBOR_MAJOR_TYPE_SIMPLE && uNumber == CBOR_SIMPLE_BREAK) {
// Break statement can be encoded in single byte too (0xff)
- uAdditionalInfo = uNumber;
+ nAdditionalInfo = (int)uNumber;
} else {
/*
Encode argument in 1,2,4 or 8 bytes. Outer loop runs once for 1
@@ -361,22 +386,38 @@
with zero bytes.
*/
static const uint8_t aIterate[] = {1,1,2,4};
- uint8_t i;
+ int i;
for(i = 0; uNumber || nMinLen > 0; i++) {
- const uint8_t uIterations = aIterate[i];
- for(int j = 0; j < uIterations; j++) {
- *--pByte = uNumber & 0xff;
+ const int nIterations = aIterate[i];
+ for(int j = 0; j < nIterations; j++) {
+ *--pByte = (uint8_t)(uNumber & 0xff);
uNumber = uNumber >> 8;
}
- nMinLen -= uIterations;
+ nMinLen -= nIterations;
}
// Additional info is the encoding of the number of additional
// bytes to encode argument.
- uAdditionalInfo = LEN_IS_ONE_BYTE-1 + i;
+ nAdditionalInfo = LEN_IS_ONE_BYTE-1 + i;
}
- *--pByte = (uMajorType << 5) + uAdditionalInfo;
- UsefulOutBuf_InsertData(&(me->OutBuf), pByte, &bytes[sizeof(bytes)-1] - pByte, uPos);
+ /*
+ Expression integer-promotes to type int. The code above in
+ function gaurantees that uAdditionalInfo will never be larger than
+ 0x1f. The caller may pass in a too-large uMajor type. The
+ conversion to unint8_t will cause an integer wrap around and
+ incorrect CBOR will be generated, but no security issue will
+ incur.
+ */
+ *--pByte = (uint8_t)((uMajorType << 5) + nAdditionalInfo);
+
+ /*
+ Will not go negative because the loops run for at most 8
+ decrements of pByte, only one other decrement is made and the
+ array is sized for this.
+ */
+ const size_t uHeadLen = (size_t)(&bytes[sizeof(bytes)-1] - pByte);
+
+ UsefulOutBuf_InsertData(&(me->OutBuf), pByte, uHeadLen, uPos);
}