Most tests fail when kFileBufferSize is set to 3 in tools/buffer.c (or the API is used directly) #324

New Issue

claunia · 2026-01-29T20:42:02Z

claunia commented

2026-01-29 20:42:02 +00:00

Originally created by @danielrh on GitHub (May 28, 2020).

I've encountered a situation where a particular personal file I have cannot be decompressed with a buffer of 8192

When I tried setting kFileBufferSize to small values like 1 2 or 3, all the tests start to fail

the offending piece of code seems to be the check for zero available_in on the context
https://github.com/google/brotli/tree/master/c/tools#L889

This is a patch to reproduce the issue
broken_buffer_size.txt

diff --git a/c/tools/brotli.c b/c/tools/brotli.c
index 04606a8..aba062d 100644
--- a/c/tools/brotli.c
+++ b/c/tools/brotli.c
@@ -792,7 +792,7 @@ static BROTLI_BOOL CloseFiles(Context* context, BROTLI_BOOL success) {
   return is_ok;
 }
 
-static const size_t kFileBufferSize = 1 << 19;
+static const size_t kFileBufferSize = 3;
 
 static void InitializeBuffers(Context* context) {
   context->available_in = 0;

Originally created by @danielrh on GitHub (May 28, 2020). I've encountered a situation where a particular personal file I have cannot be decompressed with a buffer of 8192 When I tried setting kFileBufferSize to small values like 1 2 or 3, all the tests start to fail the offending piece of code seems to be the check for zero available_in on the context https://github.com/google/brotli/tree/master/c/tools#L889 This is a patch to reproduce the issue [broken_buffer_size.txt](https://github.com/google/brotli/files/4693648/broken_buffer_size.txt) ```diff diff --git a/c/tools/brotli.c b/c/tools/brotli.c index 04606a8..aba062d 100644 --- a/c/tools/brotli.c +++ b/c/tools/brotli.c @@ -792,7 +792,7 @@ static BROTLI_BOOL CloseFiles(Context* context, BROTLI_BOOL success) { return is_ok; } -static const size_t kFileBufferSize = 1 << 19; +static const size_t kFileBufferSize = 3; static void InitializeBuffers(Context* context) { context->available_in = 0; ```

claunia closed this issue

2026-01-29 20:42:02 +00:00

claunia commented

2026-01-29 20:42:03 +00:00

@eustas commented on GitHub (Jul 2, 2020):

Strange. Internally we have tests that feed input and output byte-by-byte.
Thanks for the report. Will investigate soon.

@eustas commented on GitHub (Jul 2, 2020): Strange. Internally we have tests that feed input and output byte-by-byte. Thanks for the report. Will investigate soon.

claunia commented

2026-01-29 20:42:03 +00:00

@eustas commented on GitHub (Jul 2, 2020):

Was able to reproduce.

@eustas commented on GitHub (Jul 2, 2020): Was able to reproduce.

claunia commented

2026-01-29 20:42:03 +00:00

@eustas commented on GitHub (Jul 2, 2020):

Currently what I see is that feof is lying! It tells "no" at the end of file!
And it happens only if file size is divisible by buffer size (in this case 3).

Will check if this is a known thing and how we could do with it (e.g. do extra read instead of feof...)

@eustas commented on GitHub (Jul 2, 2020): Currently what I see is that `feof` is lying! It tells "no" at the end of file! And it happens only if file size is divisible by buffer size (in this case 3). Will check if this is a known thing and how we could do with it (e.g. do extra read instead of `feof`...)

claunia commented

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

@jfkthame commented on GitHub (Jul 2, 2020):

On 02/07/2020 15:49, Eugene Kliuchnikov wrote:

Currently what I see is that |feof| is lying! It tells "no" at the end
of file!
And it happens only if file size is divisible by buffer size (in this
case 3).

This sounds like expected behavior; feof() will return true only if
there has been an attempt to read PAST the end of file. Simply reading
UP TO the end of the file does not set the EOF flag.

JK

@jfkthame commented on GitHub (Jul 2, 2020): On 02/07/2020 15:49, Eugene Kliuchnikov wrote: > > Currently what I see is that |feof| is lying! It tells "no" at the end > of file! > And it happens only if file size is divisible by buffer size (in this > case 3). > This sounds like expected behavior; feof() will return true only if there has been an attempt to read PAST the end of file. Simply reading UP TO the end of the file does not set the EOF flag. JK

claunia commented

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

@eustas commented on GitHub (Jul 2, 2020):

Well, it is underspecified, and counterintuitive. It is not said "it will", it is said "indicator is generally set..."

@eustas commented on GitHub (Jul 2, 2020): Well, it is underspecified, and counterintuitive. It is not said "it *will*", it is said "indicator is *generally* set..."

claunia commented

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

@eustas commented on GitHub (Jul 2, 2020):

@danielrh feel free to reopen the bug, if the problem is still there.

@eustas commented on GitHub (Jul 2, 2020): @danielrh feel free to reopen the bug, if the problem is still there.

claunia commented

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

@jfkthame commented on GitHub (Jul 2, 2020):

Well, it is underspecified, and counterintuitive. It is not said "it will", it is said "indicator is generally set..."

Maybe that depends on the documentation you refer to... for example, https://en.cppreference.com/w/c/io/feof makes it clear that this is the expected behavior (although I agree it seems counterintuitive).

Anyhow, the fix looks like it should be fine.

@jfkthame commented on GitHub (Jul 2, 2020): > Well, it is underspecified, and counterintuitive. It is not said "it _will_", it is said "indicator is _generally_ set..." Maybe that depends on the documentation you refer to... for example, https://en.cppreference.com/w/c/io/feof makes it clear that this is the expected behavior (although I agree it seems counterintuitive). Anyhow, the fix looks like it should be fine.

Sign in to join this conversation.

1 Participants

Notifications

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: starred/brotli#324