Inconsistent range of lgwin values #47

New Issue

claunia · 2026-01-29T20:29:56Z

claunia commented

2026-01-29 20:29:56 +00:00

Originally created by @ericlaw1979 on GitHub (Oct 2, 2015).

In the Python version, the allowed range of the Window is 16 to 24:
PyErr_SetString(BrotliError, "Invalid lgwin. Range is 16 to 24.");

params.add_argument('--lgwin', metavar="LGWIN", type=int, choices=list(range(16, 25)), help='Base 2 logarithm of the sliding window size. Range is ' '16 to 24. Defaults to 22.')

But elsewhere, we see a different limit:

// Base 2 logarithm of the sliding window size. Range is 10 to 24. int lgwin;

if (*lgwin < 10 || *lgwin >= 25) { goto error;

Is this expected?

Originally created by @ericlaw1979 on GitHub (Oct 2, 2015). In the Python version, the allowed range of the Window is 16 to 24: ` PyErr_SetString(BrotliError, "Invalid lgwin. Range is 16 to 24."); ` ` params.add_argument('--lgwin', metavar="LGWIN", type=int, choices=list(range(16, 25)), help='Base 2 logarithm of the sliding window size. Range is ' '16 to 24. Defaults to 22.') ` But elsewhere, we see a different limit: ` // Base 2 logarithm of the sliding window size. Range is 10 to 24. int lgwin; ` ` if (*lgwin < 10 || *lgwin >= 25) { goto error; ` Is this expected?

claunia closed this issue

2026-01-29 20:29:58 +00:00

claunia commented

2026-01-29 20:30:01 +00:00

@anthrotype commented on GitHub (Oct 2, 2015):

yes, it was changed with 6511d6b016
the python extension needs to be updated accordingly.
thanks for spotting it!

@anthrotype commented on GitHub (Oct 2, 2015): yes, it was changed with 6511d6b01629631ad76864fe499a27f36d448396 the python extension needs to be updated accordingly. thanks for spotting it!

claunia commented

2026-01-29 20:30:04 +00:00

@anthrotype commented on GitHub (Oct 6, 2015):

However, the latest draft of Brotli specification still says 16 to 24 though:
https://github.com/google/brotli/blob/master/docs/draft-alakuijala-brotli-06.txt#L267

The sliding window size is a power of two, minus 16, where the power is in the range of 16 to 24.

Could anyone confirm, before I send the patch, that for --lgwin the allowed range needs to be changed from the current [16..24] to [10..24], like it says in enc/encode.h file, as per commit 6511d6b01629631ad76864fe499a27f36d448396?
Thanks.

/cc @szabadka

@anthrotype commented on GitHub (Oct 6, 2015): However, the latest draft of Brotli specification still says `16 to 24` though: https://github.com/google/brotli/blob/master/docs/draft-alakuijala-brotli-06.txt#L267 > The sliding window size is a power of two, minus 16, where the power is in the range of 16 to 24. Could anyone confirm, before I send the patch, that for `--lgwin` the allowed range needs to be changed from the current `[16..24]` to `[10..24]`, like it says in `enc/encode.h` file, as per commit 6511d6b01629631ad76864fe499a27f36d448396? Thanks. /cc @szabadka

claunia commented

2026-01-29 20:30:05 +00:00

@szabadka commented on GitHub (Oct 6, 2015):

Thanks a lot for catching this, apparently we forgot to update the introduction part of the spec. I will do that shortly.

@szabadka commented on GitHub (Oct 6, 2015): Thanks a lot for catching this, apparently we forgot to update the introduction part of the spec. I will do that shortly.

claunia commented

2026-01-29 20:30:06 +00:00

@anthrotype commented on GitHub (Oct 6, 2015):

thanks to grep ;)

@anthrotype commented on GitHub (Oct 6, 2015): thanks to `grep` ;)

claunia referenced this issue

2026-01-29 20:46:31 +00:00

[PR #47] [MERGED] Change the return value of decoder functions from integer codes to an en... #635

claunia referenced this issue

2026-01-29 20:46:32 +00:00

[PR #47] Change the return value of decoder functions from integer codes to an en... #637

Sign in to join this conversation.