Investigate and improve stream reading and writing

src/streamIO/src/streamRead.c

@@ -16,16 +16,6 @@
 #include "streamRead.h"
 #include "ioUtil.h"
 
-const unsigned char BIT_MASK[] = {	(char) 0x00,	// 0b00000000
-									(char) 0x01,	// 0b00000001
-									(char) 0x03,	// 0b00000011
-									(char) 0x07,	// 0b00000111
-									(char) 0x0F,	// 0b00001111
-									(char) 0x1F,	// 0b00011111
-									(char) 0x3F,	// 0b00111111
-									(char) 0x7F,	// 0b01111111
-									(char) 0xFF	};	// 0b11111111
-
 errorCode readNextBit(EXIStream* strm, bool* bit_val)
 {
 	if(strm->buffer.bufContent <= strm->context.bufferIndx) // the whole buffer is parsed! read another portion
@@ -49,7 +39,7 @@ errorCode readNextBit(EXIStream* strm, bool* bit_val)
 
 errorCode readBits(EXIStream* strm, unsigned char n, unsigned long* bits_val)
 {
-	unsigned int numBytesToBeRead = 1 + ((n + strm->context.bitPointer - 1) / 8);
+	unsigned int numBytesToBeRead = 1 + ((n + strm->context.bitPointer - 1) >> 3);
 	unsigned int byteIndx = 1;
 	unsigned char *buf;
 
@@ -63,21 +53,28 @@ errorCode readBits(EXIStream* strm, unsigned char n, unsigned long* bits_val)
 
 	buf = (unsigned char *) strm->buffer.buf + strm->context.bufferIndx;
 
-	*bits_val = (buf[0] & BIT_MASK[8 - strm->context.bitPointer])<<((numBytesToBeRead-1)*8);
+	// Left and right shifts clear out the used bits.
+	// Replaced (* 8) with (<< 3) to avoid slow multiplication.
+	// Cast to unsigned long prevents sign-extension if bits shift into the sign slot.
+	unsigned int shiftAmount = (numBytesToBeRead << 3) - 8;
+	*bits_val = (unsigned long)(((buf[0] << strm->context.bitPointer) & 0xFF) >> strm->context.bitPointer);
+	*bits_val = *bits_val << shiftAmount;
 
+	// Deduct 8 from the shiftAmount register on each loop instead of recalculating multiplication.
 	while(byteIndx < numBytesToBeRead)
 	{
-		*bits_val += (unsigned long) (buf[byteIndx])<<((numBytesToBeRead-byteIndx-1)*8);
+		shiftAmount -= 8;
+		*bits_val += (unsigned long)(buf[byteIndx]) << shiftAmount;
 		byteIndx++;
 	}
 
-	*bits_val = *bits_val >> (numBytesToBeRead*8 - n - strm->context.bitPointer);
+	*bits_val = *bits_val >> ((numBytesToBeRead << 3) - n - strm->context.bitPointer);
 
 	DEBUG_MSG(INFO, DEBUG_STREAM_IO, (">> %lu [0x%lX] (%u bits)", *bits_val, *bits_val, n));
 
 	n += strm->context.bitPointer;
-	strm->context.bufferIndx += n / 8;
-	strm->context.bitPointer = n % 8;
+	strm->context.bufferIndx += n >> 3;
+	strm->context.bitPointer = n & 7;
 
 	DEBUG_MSG(INFO, DEBUG_STREAM_IO, ("  @%u:%u\n", (unsigned int) strm->context.bufferIndx, strm->context.bitPointer));
 

src/streamIO/src/streamWrite.c

@@ -16,8 +16,6 @@
 #include "streamWrite.h"
 #include "ioUtil.h"
 
-extern const unsigned char BIT_MASK[];
-
 errorCode writeNextBit(EXIStream* strm, bool bit_val)
 {
 	if(strm->buffer.bufLen <= strm->context.bufferIndx) // the whole buffer is filled! flush it!
@@ -47,7 +45,8 @@ errorCode writeNBits(EXIStream* strm, unsigned char nbits, unsigned long bits_va
 	unsigned int numBitsWrite = 0; // Number of the bits written so far
 	unsigned char tmp = 0;
 	int bits_in_byte = 0; // Number of bits written in one iteration
-	unsigned int numBytesToBeWritten = ((unsigned int) nbits) / 8 + (8 - strm->context.bitPointer < nbits % 8 );
+	// Replaced (* 8) with (<< 3) and replaced (nbits & 7) with (nbits % 8)
+	unsigned int numBytesToBeWritten = (((unsigned int) nbits) >> 3) + (8 - strm->context.bitPointer < (nbits & 7));
 
 	if(strm->buffer.bufLen <= strm->context.bufferIndx + numBytesToBeWritten)
 	{
@@ -65,9 +64,13 @@ errorCode writeNBits(EXIStream* strm, unsigned char nbits, unsigned long bits_va
 		else // The rest of the unwritten bits are more than the bits in the current byte from the stream
 			bits_in_byte = 8 - strm->context.bitPointer;
 
-		tmp = (bits_val >> (nbits - numBitsWrite - bits_in_byte)) & BIT_MASK[bits_in_byte];
+		// Creates a mask of 1s on the fly by shifting 1 and subtracting 1
+		unsigned int mask = (1U << bits_in_byte) - 1;
+		tmp = (unsigned char)((bits_val >> (nbits - numBitsWrite - bits_in_byte)) & mask);
 		tmp = tmp << (8 - strm->context.bitPointer - bits_in_byte);
-		strm->buffer.buf[strm->context.bufferIndx] = strm->buffer.buf[strm->context.bufferIndx] & (~BIT_MASK[8 - strm->context.bitPointer]); // Initialize the unused bits with 0s
+		// Creates an eraser mask to clear unused bit slots without 8-bit overflow
+		unsigned int clear_mask = 0xFFFFFFFFU << (8 - strm->context.bitPointer);
+		strm->buffer.buf[strm->context.bufferIndx] = strm->buffer.buf[strm->context.bufferIndx] & clear_mask;
 		strm->buffer.buf[strm->context.bufferIndx] = strm->buffer.buf[strm->context.bufferIndx] | tmp;
 
 		numBitsWrite += bits_in_byte;

tests/test-set/simpleContentExt/README.md

@@ -2,7 +2,7 @@
 
 Attempted reproducer for a grammar collision issue found in CCSDS NDM/XML 4.0 schemas.
 
-### Real-World Issue
+### Original Issue (Not Reproduced)
 
 CCSDS NDM/XML 4.0 schemas trigger a grammar collision error during schema generation:
 
@@ -12,16 +12,30 @@ CCSDS NDM/XML 4.0 schemas trigger a grammar collision error during schema genera
 
 The schemas return `EXIP_NOT_IMPLEMENTED_YET` (error code 1) and cannot be loaded.
 
-### Reproducer Status
+**Reproducer Status:** Incomplete - the minimal schema here does not trigger the collision. Schema loads successfully with `EXIP_OK`.
 
-Incomplete - the minimal schema here does **not** trigger the collision. The `test_sequence_unbounded` test in `check_strict_grammar` currently passes (schema loads successfully with `EXIP_OK`).
+### Issue Actually Found and Fixed
+
+While attempting to reproduce the sequence unbounded issue, a **different bug was discovered**:
+
+**Problem:** Schema loaded successfully but schema-informed decoding failed on empty elements with simpleContent extensions.
+
+**Symptoms:**
+- Schema generation: EXIP_OK
+- Schemaless decoding: Works fine
+- Schema-informed decoding: Failed with buffer underrun after empty element `<USER_DEFINED parameter="DECAY_DATE"/>`
+
+**Root Cause:** The `isContent2Grammar` logic in `bodyDecode.c` only handled the case where `currNonTermID == contentIndex == 0`. For simpleContent extensions where content is at NT-1, the equality case `1 == 1` was missing.
+
+**Fix:** Added general equality check in `bodyDecode.c` to handle `currNonTermID == contentIndex` for any value, ensuring proper second-level production handling.
 
 ### Test Files
 
 - `test.xsd` - Minimal schema attempting to reproduce the pattern
 - `test.xsd.exi` - EXI-encoded schema
 - `test.xml` - Sample data
-- `test.xml.exi` - EXI-encoded data
+- `test.xml.exi` - EXI-encoded data (schema-informed)
+- `test.xml.sl.exi` - EXI-encoded data (schemaless)
 
 ### Full Test Command
 
@@ -31,20 +45,20 @@ Currently the test mentioned above does not decode the `test.xml.exi` file but w
 Schema-informed and schemaless respectively.
 
 ```sh
-./build/vs2022/Debug/exipd.exe -xml -schema=tests/test-set/sequenceUnbounded/test.xsd.exi tests/test-set/sequenceUnbounded/test.xml.exi
+./build/vs2022/Debug/exipd.exe -xml -schema=tests/test-set/simpleContentExt/test.xsd.exi tests/test-set/simpleContentExt/test.xml.exi
 
-./build/vs2022/Debug/exipd.exe -xml tests/test-set/sequenceUnbounded/test.xml.sl.exi
+./build/vs2022/Debug/exipd.exe -xml tests/test-set/simpleContentExt/test.xml.sl.exi
 ```
 
 **Linux/macOS:**
 
 ```sh
-./bin/examples/exipd -xml -schema=tests/test-set/sequenceUnbounded/test.xsd.exi tests/test-set/sequenceUnbounded/test.xml.exi
+./bin/examples/exipd -xml -schema=tests/test-set/simpleContentExt/test.xsd.exi tests/test-set/simpleContentExt/test.xml.exi
 ```
 
 **Grammar:**
 
 Use the path above for Windows, just replace to `exipg.exe`.
 ```sh
-./bin/utils/exipg -text -schema=tests/test-set/sequenceUnbounded/test.xsd.exi
+./bin/utils/exipg -text -schema=tests/test-set/simpleContentExt/test.xsd.exi
 ```

utils/bindGen/exipb.c

@@ -189,6 +189,7 @@ int main(int argc, char *argv[])
 
     printf("Success! Generated %u TreeTable(s)\n", treeTCount);
     printf("Schema has %zu namespaces\n", schema.uriTable.count);
+    printf("Schema has %zu namespaces\n", schema.uriTable.count);
 
 #if DEBUG_BINDING == ON
     // Print tree table structure for inspection (debug output only)