Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions doc/educational_decoder/harness.c
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ static buffer_s read_file(const char *path)

fclose(f);
buffer_s const b = { ptr, size };
free(ptr);
Copy link
Contributor

@Cyan4973 Cyan4973 Apr 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This change is incorrect :
the whole point of this function is to return a populated buffer, passed as a member of the buffer_s structure, effectively transferring ownership to the caller of the function (which will have to free it later, using the provided freeBuffer() function).

Maybe this could be documented if it's not clear enough...

return b;
}

Expand Down
11 changes: 8 additions & 3 deletions tests/bigdict.c
Original file line number Diff line number Diff line change
Expand Up @@ -70,12 +70,14 @@ int main(int argc, const char** argv)
char* buffer = (char*)malloc(bufferSize);
void* out = malloc(outSize);
void* roundtrip = malloc(dataSize);
int _exit_code = 0;
(void)argc;
(void)argv;

if (!buffer || !out || !roundtrip || !cctx || !dctx) {
fprintf(stderr, "Allocation failure\n");
return 1;
_exit_code = 1;
goto cleanup;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

}

if (ZSTD_isError(ZSTD_CCtx_setParameter(cctx, ZSTD_c_windowLog, 31)))
Expand Down Expand Up @@ -119,10 +121,13 @@ int main(int argc, const char** argv)

fprintf(stderr, "Success!\n");

goto cleanup;

cleanup:
free(roundtrip);
free(out);
free(buffer);
ZSTD_freeDCtx(dctx);
ZSTD_freeCCtx(cctx);
return 0;
ZSTD_freeDCtx(dctx);
return _exit_code;
}
1 change: 0 additions & 1 deletion tests/paramgrill.c
Original file line number Diff line number Diff line change
Expand Up @@ -1273,7 +1273,6 @@ static int createBuffers(buffers_t* buff, const char* const * const fileNamesTab
f = fopen(fileNamesTable[n], "rb");
if (f==NULL) {
DISPLAY("impossible to open file %s\n", fileNamesTable[n]);
fclose(f);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍

ret = 10;
goto _cleanUp;
}
Expand Down
2 changes: 2 additions & 0 deletions tests/regression/result.c
Original file line number Diff line number Diff line change
Expand Up @@ -24,5 +24,7 @@ char const* result_get_error_string(result_t result) {
return "decompression error";
case result_error_round_trip_error:
return "round trip error";
default:
return "unknown error - " + result_get_error(result);
}
}
4 changes: 2 additions & 2 deletions zlibWrapper/examples/minigzip.c
Original file line number Diff line number Diff line change
Expand Up @@ -234,7 +234,7 @@ int gzwrite _Z_OF((gzFile, const void *, unsigned));

int gzwrite(gzFile gz, const void *buf, unsigned len) {
z_stream *strm;
unsigned char out[BUFLEN];
unsigned char out[BUFLEN] = 0;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you meant { 0 } ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

God damn, I'm absent-minded...
Sorry I made so much mistakes in something that's supposed to fix stuff...

Will fix this though, thanks for noticing


if (gz == NULL || !gz->write)
return 0;
Expand Down Expand Up @@ -287,7 +287,7 @@ int gzclose _Z_OF((gzFile));

int gzclose(gzFile gz) {
z_stream *strm;
unsigned char out[BUFLEN];
unsigned char out[BUFLEN] = 0;

if (gz == NULL)
return Z_STREAM_ERROR;
Expand Down
2 changes: 2 additions & 0 deletions zlibWrapper/gzwrite.c
Original file line number Diff line number Diff line change
Expand Up @@ -64,6 +64,8 @@ local int gz_init(gz_statep state) {
strm->next_out = state.state->out;
state.state->x.next = strm->next_out;
}

free(state.state);
Copy link
Contributor

@Cyan4973 Cyan4973 Apr 12, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That one is weird.
I'm not even sure what's going on in this code.

One of the first actions in this function is : state.state->in = (unsigned char*)malloc(state.state->want << 1);,
which presumes that state.state is already allocated (should probably be asserted), before entering the function,
which means that, something else has allocated state.state, and therefore something else is in charge of freeing it.
I don't see how it could be good to free it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, You're right. That was an oversight from me.
I'll fix this tomorrow morning... Again, sorry for the inconvenience

return 0;
}

Expand Down