diff --git a/src/streamIO/src/streamRead.c b/src/streamIO/src/streamRead.c index ad8ebad..d2d3fcd 100644 --- a/src/streamIO/src/streamRead.c +++ b/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)); diff --git a/src/streamIO/src/streamWrite.c b/src/streamIO/src/streamWrite.c index e09e126..1386519 100644 --- a/src/streamIO/src/streamWrite.c +++ b/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; diff --git a/tests/test-set/simpleContentExt/README.md b/tests/test-set/simpleContentExt/README.md index 025b496..4d6a997 100644 --- a/tests/test-set/simpleContentExt/README.md +++ b/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 `` + +**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 ``` diff --git a/utils/bindGen/exipb.c b/utils/bindGen/exipb.c index b88067b..6185bf7 100644 --- a/utils/bindGen/exipb.c +++ b/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)