Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Check for failing mallocs #40

Closed
wants to merge 1 commit into from

Conversation

LennartNoordsij
Copy link
Contributor

malloc can fail and return a NULL pointer. Previously this was unchecked.
This commit adds a check for that.

malloc can fail and return a NULL pointer. Previously this was unchecked.
This commit adds a check for that.
@@ -49,9 +49,17 @@ compress_init_par(zfp_stream* stream, const zfp_field* field, uint chunks, uint

/* set up buffer for each thread to compress to */
bs = (bitstream**)malloc(chunks * sizeof(bitstream*));
if (!bs) {
fprintf(stderr, "cannot allocate memory for per-thread bit streams\n");
exit(EXIT_FAILURE);
Copy link
Member

Choose a reason for hiding this comment

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

I'm afraid that exiting the entire application on failure is not acceptable. zfp also should not write to stderr. We need to return NULL here and have the caller check the return value.

for (i = 0; i < chunks; i++) {
uint block = chunk_offset(blocks, chunks, i);
void* buffer = copy ? malloc(size) : (uchar*)stream_data(stream->stream) + stream_size(stream->stream) + block * stream->maxbits / CHAR_BIT;
if (copy && !buffer) {
fprintf(stderr, "cannot allocate memory for compression buffer\n");
exit(EXIT_FAILURE);
Copy link
Member

Choose a reason for hiding this comment

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

Another case where exiting is not a viable option. We need to clean up after ourselves and then return NULL.

@lindstro
Copy link
Member

@LennartNoordsij Thanks for pointing out this issue and for your contribution. I fear that this PR requires additional work, since terminating the calling application when memory allocation fails is not a viable solution. Doing this correctly requires additional code to free any partially allocated resources, reporting an error back to the caller, and having the caller take appropriate action.

I have pushed a revised version to the develop branch and am closing this PR.

@lindstro lindstro closed this Mar 31, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants