enc/dec: add BROTLI_DCHECKs for size computation bounds (per @eustas review)

[SongT-50]

Following @eustas's review of #1471: replace early-return overflow guards with
BROTLI_DCHECK assertions. This preserves the intent (document invariants) while
avoiding any runtime control-flow impact and the catastrophic-return concern.

Per reviewer suggestion: "If any of limits are hard to track, prefer BROTLI_DCHECK
to ensure them."

Changes

c/enc/block_splitter_inc.h — FN(SplitByteVector) (2 checks):

• data_size * num_histograms multiplication bound
• length * bitmaplen multiplication bound

c/enc/block_splitter_inc.h — FN(ClusterBlocks) (1 check):

• num_blocks + 4 * HISTOGRAMS_PER_BATCH addition bound

c/enc/block_splitter.c — CopyLiteralsToByteArray (1 check):

• insert_len + from_pos wrap guard

c/enc/block_encoder_inc.h — FN(BuildAndStoreEntropyCodes) (1 check):

• histograms_size * histogram_length_ multiplication bound

c/dec/state.c — BrotliDecoderHuffmanTreeGroupInit (3 checks):

• alphabet_size_limit + 376 addition bound
• ntrees * sizeof(HuffmanCode) * max_table_size multiplication bound
• ntrees * sizeof(HuffmanCode*) multiplication bound

Build

Debug build (cmake -DCMAKE_BUILD_TYPE=Debug) passes without warnings or errors.

[eustas]

Sorry, I should have been more explicit - BROTLI_DCHECK declares some readable fact, e.g. "we know/expect that alphabet size is not more than this constant"; in this PR BROTLI_DCHECK checks that overwflow should not happen - that does not help reader to understand why.

[SongT-50]

Thank you for the clarification. I have updated PR v3 to use declarative-invariant form per your guidance, with bounds taken directly from your inline review of #1471:

• data_size <= 704, num_histograms <= 100, bitmaplen <= 13, length <= 16M (block_splitter_inc.h SplitByteVector)
• num_blocks <= 16M (block_splitter_inc.h ClusterBlocks)
• histograms_size <= 257, histogram_length_ <= 704 (block_encoder_inc.h BuildAndStoreEntropyCodes)
• alphabet_size_limit <= BROTLI_NUM_DISTANCE_SYMBOLS (== 1128 per RFC 7932), max_table_size <= 1504 (== 1128 + 376), ntrees < 1024 (dec/state.c BrotliDecoderHuffmanTreeGroupInit)

The block_splitter.c BROTLI_DCHECK is removed since you noted the situation is not possible; it is replaced with an explanatory comment.

Notes:

• The names BROTLI_MAX_ALPHABET_SIZE and BROTLI_HUFFMAN_MAX_TABLE_SIZE are not present in the tree, so literal numeric bounds with explanatory comments are used in the corresponding slots.
• A single ntrees < 1024 DCHECK at line 172 covers both the code_size and htree_size multiplications below.

Debug build (cmake -DCMAKE_BUILD_TYPE=Debug) succeeds with no warnings or errors.

[eustas]

Awesome. One more thing - we stick to C89 syntax compatibility. So BROTLI_DCHECK is not allowed before all variable declarations. I see you have added nice explanatory comments. Since we have covered why situation is safe, we could drop BROTLI_DCHECK and leave only comments. Thanks.

[SongT-50]

Thanks for the C89 reminder! Removed BROTLI_DCHECK and kept the explanatory comments as you suggested. Force-pushed.

[SongT-50]

Hi @eustas,

Just a gentle follow-up on PR #1472 — wanted to check whether there's anything else you'd like me to address on the C89-compatible comments approach we landed on.

If the current state looks good, no action needed on my end. If there are any concerns about CI status (mergeable shows UNKNOWN), I'm happy to investigate.

Thanks for the careful review on this one — the back-and-forth on declarative-invariant form and C89 compatibility was very educational.

Best,
Tae-Eun

[SongT-50]

Hi @eustas — a gentle ping on this one. The DCHECKs are updated per your earlier review. No rush at all, but happy to rebase or address any remaining concerns whenever you have a moment. Thanks!