Skip to content

Commit 78735ff

Browse files
committed
Rewrite readBitsInt{Be,Le}()
See kaitai-io/kaitai_struct#949
1 parent 358caf1 commit 78735ff

File tree

1 file changed

+32
-28
lines changed

1 file changed

+32
-28
lines changed

lib/Kaitai/Struct/Stream.php

Lines changed: 32 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,8 @@ class Stream {
1414
const SIGN_MASK_16 = 0x8000; // (1 << (16 - 1));
1515
const SIGN_MASK_32 = 0x80000000; // (1 << (32 - 1));
1616

17-
private $bits, $bitsLeft;
17+
private $bitsLeft;
18+
private $bits;
1819

1920
/**
2021
* @param resource|string $stream
@@ -218,34 +219,34 @@ public function readF8le(): float {
218219
**************************************************************************/
219220

220221
public function alignToByte()/*: void */ {
221-
$this->bits = 0;
222222
$this->bitsLeft = 0;
223+
$this->bits = 0;
223224
}
224225

225226
public function readBitsIntBe(int $n): int {
227+
$res = 0;
228+
226229
$bitsNeeded = $n - $this->bitsLeft;
230+
$this->bitsLeft = -$bitsNeeded & 7; // `-$bitsNeeded mod 8`
231+
227232
if ($bitsNeeded > 0) {
228233
// 1 bit => 1 byte
229234
// 8 bits => 1 byte
230235
// 9 bits => 2 bytes
231-
$bytesNeeded = intdiv($bitsNeeded - 1, 8) + 1;
236+
$bytesNeeded = (($bitsNeeded - 1) >> 3) + 1; // `ceil($bitsNeeded / 8)` (NB: `x >> 3` is `floor(x / 8)`)
232237
$buf = $this->readBytes($bytesNeeded);
233238
for ($i = 0; $i < $bytesNeeded; $i++) {
234-
$b = ord($buf[$i]);
235-
$this->bits <<= 8;
236-
$this->bits |= $b;
237-
$this->bitsLeft += 8;
239+
$res = $res << 8 | ord($buf[$i]);
238240
}
241+
242+
$newBits = $res;
243+
$res = self::zeroFillRightShift($res, $this->bitsLeft) | $this->bits << $bitsNeeded;
244+
$this->bits = $newBits; // will be masked at the end of the function
245+
} else {
246+
$res = self::zeroFillRightShift($this->bits, -$bitsNeeded); // shift unneeded bits out
239247
}
240248

241-
// raw mask with required number of 1s, starting from lowest bit
242-
$mask = $this->getMaskOnes($n);
243-
// shift $this->bits to align the highest bits with the mask & derive reading result
244-
$shiftBits = $this->bitsLeft - $n;
245-
$res = ($this->bits >> $shiftBits) & $mask;
246-
// clear top bits that we've just read => AND with 1s
247-
$this->bitsLeft -= $n;
248-
$mask = (1 << $this->bitsLeft) - 1;
249+
$mask = (1 << $this->bitsLeft) - 1; // `bitsLeft` is in range 0..7, so `(1 << 63)` does not have to be considered
249250
$this->bits &= $mask;
250251

251252
return $res;
@@ -261,36 +262,39 @@ public function readBitsInt(int $n): int {
261262
}
262263

263264
public function readBitsIntLe(int $n): int {
265+
$res = 0;
264266
$bitsNeeded = $n - $this->bitsLeft;
267+
265268
if ($bitsNeeded > 0) {
266269
// 1 bit => 1 byte
267270
// 8 bits => 1 byte
268271
// 9 bits => 2 bytes
269-
$bytesNeeded = intdiv($bitsNeeded - 1, 8) + 1;
272+
$bytesNeeded = (($bitsNeeded - 1) >> 3) + 1; // `ceil($bitsNeeded / 8)` (NB: `x >> 3` is `floor(x / 8)`)
270273
$buf = $this->readBytes($bytesNeeded);
271274
for ($i = 0; $i < $bytesNeeded; $i++) {
272-
$b = ord($buf[$i]);
273-
$this->bits |= ($b << $this->bitsLeft);
274-
$this->bitsLeft += 8;
275+
$res |= ord($buf[$i]) << ($i * 8);
275276
}
277+
278+
$newBits = self::zeroFillRightShift($res, $bitsNeeded);
279+
$res = $res << $this->bitsLeft | $this->bits;
280+
$this->bits = $newBits;
281+
} else {
282+
$res = $this->bits;
283+
$this->bits = self::zeroFillRightShift($this->bits, $n);
276284
}
277285

278-
// raw mask with required number of 1s, starting from lowest bit
279-
$mask = $this->getMaskOnes($n);
280-
// derive reading result
281-
$res = $this->bits & $mask;
282-
// remove bottom bits that we've just read by shifting
283-
$this->bits = $this->zeroFillRightShift($this->bits, $n);
284-
$this->bitsLeft -= $n;
286+
$this->bitsLeft = -$bitsNeeded & 7; // `-$bitsNeeded mod 8`
285287

288+
$mask = self::getMaskOnes($n);
289+
$res &= $mask;
286290
return $res;
287291
}
288292

289293
private static function getMaskOnes(int $n): int {
290294
// 1. (1 << 63) === PHP_INT_MIN (and yes, it is negative, because PHP uses signed 64-bit ints on 64-bit system),
291295
// so (1 << 63) - 1 gets converted to float and loses precision (leading to incorrect result)
292296
// 2. (1 << 64) - 1 works fine, because (1 << 64) === 0 (it overflows) and -1 is exactly what we want
293-
// (`php -r "var_dump(decbin(-1));"` => string(64) "111...11")
297+
// (`php -r 'var_dump(decbin(-1));'` => string(64) "111...11")
294298
$bit = 1 << $n;
295299
return $bit === PHP_INT_MIN ? ~$bit : $bit - 1;
296300
}
@@ -484,7 +488,7 @@ private static function decodeSignedInt(int $x, int $mask): int {
484488
private static function zeroFillRightShift(int $a, int $b): int {
485489
$res = $a >> $b;
486490
if ($a >= 0 || $b === 0) return $res;
487-
return $res & ~(PHP_INT_MIN >> ($b - 1));
491+
return $res & (PHP_INT_MAX >> ($b - 1));
488492
}
489493

490494
private function decodeSinglePrecisionFloat(int $bits): float {

0 commit comments

Comments
 (0)