Skip to content

Commit 3c7f437

Browse files
authored
Investigate and improve stream reading and writing (#99)
* Replace modulo, multiply, divide, and mask lookup with bit shifting in readbits * Replace modulo, multiply, divide, and mask lookup with bit shifting in writeNBits * Fix printf type * Update README for previous merge
1 parent ba996cf commit 3c7f437

4 files changed

Lines changed: 44 additions & 29 deletions

File tree

src/streamIO/src/streamRead.c

Lines changed: 13 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -16,16 +16,6 @@
1616
#include "streamRead.h"
1717
#include "ioUtil.h"
1818

19-
const unsigned char BIT_MASK[] = { (char) 0x00, // 0b00000000
20-
(char) 0x01, // 0b00000001
21-
(char) 0x03, // 0b00000011
22-
(char) 0x07, // 0b00000111
23-
(char) 0x0F, // 0b00001111
24-
(char) 0x1F, // 0b00011111
25-
(char) 0x3F, // 0b00111111
26-
(char) 0x7F, // 0b01111111
27-
(char) 0xFF }; // 0b11111111
28-
2919
errorCode readNextBit(EXIStream* strm, bool* bit_val)
3020
{
3121
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)
4939

5040
errorCode readBits(EXIStream* strm, unsigned char n, unsigned long* bits_val)
5141
{
52-
unsigned int numBytesToBeRead = 1 + ((n + strm->context.bitPointer - 1) / 8);
42+
unsigned int numBytesToBeRead = 1 + ((n + strm->context.bitPointer - 1) >> 3);
5343
unsigned int byteIndx = 1;
5444
unsigned char *buf;
5545

@@ -63,21 +53,28 @@ errorCode readBits(EXIStream* strm, unsigned char n, unsigned long* bits_val)
6353

6454
buf = (unsigned char *) strm->buffer.buf + strm->context.bufferIndx;
6555

66-
*bits_val = (buf[0] & BIT_MASK[8 - strm->context.bitPointer])<<((numBytesToBeRead-1)*8);
56+
// Left and right shifts clear out the used bits.
57+
// Replaced (* 8) with (<< 3) to avoid slow multiplication.
58+
// Cast to unsigned long prevents sign-extension if bits shift into the sign slot.
59+
unsigned int shiftAmount = (numBytesToBeRead << 3) - 8;
60+
*bits_val = (unsigned long)(((buf[0] << strm->context.bitPointer) & 0xFF) >> strm->context.bitPointer);
61+
*bits_val = *bits_val << shiftAmount;
6762

63+
// Deduct 8 from the shiftAmount register on each loop instead of recalculating multiplication.
6864
while(byteIndx < numBytesToBeRead)
6965
{
70-
*bits_val += (unsigned long) (buf[byteIndx])<<((numBytesToBeRead-byteIndx-1)*8);
66+
shiftAmount -= 8;
67+
*bits_val += (unsigned long)(buf[byteIndx]) << shiftAmount;
7168
byteIndx++;
7269
}
7370

74-
*bits_val = *bits_val >> (numBytesToBeRead*8 - n - strm->context.bitPointer);
71+
*bits_val = *bits_val >> ((numBytesToBeRead << 3) - n - strm->context.bitPointer);
7572

7673
DEBUG_MSG(INFO, DEBUG_STREAM_IO, (">> %lu [0x%lX] (%u bits)", *bits_val, *bits_val, n));
7774

7875
n += strm->context.bitPointer;
79-
strm->context.bufferIndx += n / 8;
80-
strm->context.bitPointer = n % 8;
76+
strm->context.bufferIndx += n >> 3;
77+
strm->context.bitPointer = n & 7;
8178

8279
DEBUG_MSG(INFO, DEBUG_STREAM_IO, (" @%u:%u\n", (unsigned int) strm->context.bufferIndx, strm->context.bitPointer));
8380

src/streamIO/src/streamWrite.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,6 @@
1616
#include "streamWrite.h"
1717
#include "ioUtil.h"
1818

19-
extern const unsigned char BIT_MASK[];
20-
2119
errorCode writeNextBit(EXIStream* strm, bool bit_val)
2220
{
2321
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
4745
unsigned int numBitsWrite = 0; // Number of the bits written so far
4846
unsigned char tmp = 0;
4947
int bits_in_byte = 0; // Number of bits written in one iteration
50-
unsigned int numBytesToBeWritten = ((unsigned int) nbits) / 8 + (8 - strm->context.bitPointer < nbits % 8 );
48+
// Replaced (* 8) with (<< 3) and replaced (nbits & 7) with (nbits % 8)
49+
unsigned int numBytesToBeWritten = (((unsigned int) nbits) >> 3) + (8 - strm->context.bitPointer < (nbits & 7));
5150

5251
if(strm->buffer.bufLen <= strm->context.bufferIndx + numBytesToBeWritten)
5352
{
@@ -65,9 +64,13 @@ errorCode writeNBits(EXIStream* strm, unsigned char nbits, unsigned long bits_va
6564
else // The rest of the unwritten bits are more than the bits in the current byte from the stream
6665
bits_in_byte = 8 - strm->context.bitPointer;
6766

68-
tmp = (bits_val >> (nbits - numBitsWrite - bits_in_byte)) & BIT_MASK[bits_in_byte];
67+
// Creates a mask of 1s on the fly by shifting 1 and subtracting 1
68+
unsigned int mask = (1U << bits_in_byte) - 1;
69+
tmp = (unsigned char)((bits_val >> (nbits - numBitsWrite - bits_in_byte)) & mask);
6970
tmp = tmp << (8 - strm->context.bitPointer - bits_in_byte);
70-
strm->buffer.buf[strm->context.bufferIndx] = strm->buffer.buf[strm->context.bufferIndx] & (~BIT_MASK[8 - strm->context.bitPointer]); // Initialize the unused bits with 0s
71+
// Creates an eraser mask to clear unused bit slots without 8-bit overflow
72+
unsigned int clear_mask = 0xFFFFFFFFU << (8 - strm->context.bitPointer);
73+
strm->buffer.buf[strm->context.bufferIndx] = strm->buffer.buf[strm->context.bufferIndx] & clear_mask;
7174
strm->buffer.buf[strm->context.bufferIndx] = strm->buffer.buf[strm->context.bufferIndx] | tmp;
7275

7376
numBitsWrite += bits_in_byte;

tests/test-set/simpleContentExt/README.md

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@
22

33
Attempted reproducer for a grammar collision issue found in CCSDS NDM/XML 4.0 schemas.
44

5-
### Real-World Issue
5+
### Original Issue (Not Reproduced)
66

77
CCSDS NDM/XML 4.0 schemas trigger a grammar collision error during schema generation:
88

@@ -12,16 +12,30 @@ CCSDS NDM/XML 4.0 schemas trigger a grammar collision error during schema genera
1212

1313
The schemas return `EXIP_NOT_IMPLEMENTED_YET` (error code 1) and cannot be loaded.
1414

15-
### Reproducer Status
15+
**Reproducer Status:** Incomplete - the minimal schema here does not trigger the collision. Schema loads successfully with `EXIP_OK`.
1616

17-
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`).
17+
### Issue Actually Found and Fixed
18+
19+
While attempting to reproduce the sequence unbounded issue, a **different bug was discovered**:
20+
21+
**Problem:** Schema loaded successfully but schema-informed decoding failed on empty elements with simpleContent extensions.
22+
23+
**Symptoms:**
24+
- Schema generation: EXIP_OK
25+
- Schemaless decoding: Works fine
26+
- Schema-informed decoding: Failed with buffer underrun after empty element `<USER_DEFINED parameter="DECAY_DATE"/>`
27+
28+
**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.
29+
30+
**Fix:** Added general equality check in `bodyDecode.c` to handle `currNonTermID == contentIndex` for any value, ensuring proper second-level production handling.
1831

1932
### Test Files
2033

2134
- `test.xsd` - Minimal schema attempting to reproduce the pattern
2235
- `test.xsd.exi` - EXI-encoded schema
2336
- `test.xml` - Sample data
24-
- `test.xml.exi` - EXI-encoded data
37+
- `test.xml.exi` - EXI-encoded data (schema-informed)
38+
- `test.xml.sl.exi` - EXI-encoded data (schemaless)
2539

2640
### Full Test Command
2741

@@ -31,20 +45,20 @@ Currently the test mentioned above does not decode the `test.xml.exi` file but w
3145
Schema-informed and schemaless respectively.
3246

3347
```sh
34-
./build/vs2022/Debug/exipd.exe -xml -schema=tests/test-set/sequenceUnbounded/test.xsd.exi tests/test-set/sequenceUnbounded/test.xml.exi
48+
./build/vs2022/Debug/exipd.exe -xml -schema=tests/test-set/simpleContentExt/test.xsd.exi tests/test-set/simpleContentExt/test.xml.exi
3549

36-
./build/vs2022/Debug/exipd.exe -xml tests/test-set/sequenceUnbounded/test.xml.sl.exi
50+
./build/vs2022/Debug/exipd.exe -xml tests/test-set/simpleContentExt/test.xml.sl.exi
3751
```
3852

3953
**Linux/macOS:**
4054

4155
```sh
42-
./bin/examples/exipd -xml -schema=tests/test-set/sequenceUnbounded/test.xsd.exi tests/test-set/sequenceUnbounded/test.xml.exi
56+
./bin/examples/exipd -xml -schema=tests/test-set/simpleContentExt/test.xsd.exi tests/test-set/simpleContentExt/test.xml.exi
4357
```
4458

4559
**Grammar:**
4660

4761
Use the path above for Windows, just replace to `exipg.exe`.
4862
```sh
49-
./bin/utils/exipg -text -schema=tests/test-set/sequenceUnbounded/test.xsd.exi
63+
./bin/utils/exipg -text -schema=tests/test-set/simpleContentExt/test.xsd.exi
5064
```

utils/bindGen/exipb.c

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -189,6 +189,7 @@ int main(int argc, char *argv[])
189189

190190
printf("Success! Generated %u TreeTable(s)\n", treeTCount);
191191
printf("Schema has %zu namespaces\n", schema.uriTable.count);
192+
printf("Schema has %zu namespaces\n", schema.uriTable.count);
192193

193194
#if DEBUG_BINDING == ON
194195
// Print tree table structure for inspection (debug output only)

0 commit comments

Comments
 (0)