-
Notifications
You must be signed in to change notification settings - Fork 29
Increase code sanity and test coverage #42
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
Open
crusaderky
wants to merge
1
commit into
Blosc:master
Choose a base branch
from
crusaderky:sanity_and_tests
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ | |
| */ | ||
|
|
||
|
|
||
| #include <assert.h> | ||
| #include <stdlib.h> | ||
| #include <stdio.h> | ||
| #include <string.h> | ||
|
|
@@ -28,8 +29,6 @@ | |
| #define PUSH_ERR(func, minor, ...) H5Epush(H5E_DEFAULT, __FILE__, func, __LINE__, H5E_ERR_CLS, H5E_PLINE, minor, __VA_ARGS__) | ||
| #endif /* defined(__GNUC__) */ | ||
|
|
||
| #define GET_FILTER(a, b, c, d, e, f, g) H5Pget_filter_by_id(a,b,c,d,e,f,g,NULL) | ||
|
|
||
|
|
||
| size_t blosc_filter(unsigned flags, size_t cd_nelmts, | ||
| const unsigned cd_values[], size_t nbytes, | ||
|
|
@@ -79,23 +78,34 @@ herr_t blosc_set_local(hid_t dcpl, hid_t type, hid_t space) { | |
| int i; | ||
| herr_t r; | ||
|
|
||
| unsigned int typesize, basetypesize; | ||
| unsigned int bufsize; | ||
| unsigned int typesize, chunksize, basetypesize; | ||
| hsize_t chunkdims[32]; | ||
| unsigned int flags; | ||
| size_t nelements = 8; | ||
| unsigned int values[] = {0, 0, 0, 0, 0, 0, 0, 0}; | ||
| size_t cd_nelmts = 8; | ||
| /* | ||
| * cd_values[0] = hdf5-blosc format version | ||
| * cd_values[1] = blosc format version | ||
| * cd_values[2] = typesize | ||
| * cd_values[3] = uncompressed chunk size (unused) | ||
| * cd_values[4] = compression level | ||
| * cd_values[5] = 0: shuffle not active, 1: shuffle active | ||
| * cd_values[6] = compressor, e.g. BLOSC_BLOSCLZ | ||
| * cd_values[7] = unused | ||
| */ | ||
| unsigned int cd_values[] = {0, 0, 0, 0, 0, 0, 0, 0}; | ||
| hid_t super_type; | ||
| H5T_class_t classt; | ||
|
|
||
| r = GET_FILTER(dcpl, FILTER_BLOSC, &flags, &nelements, values, 0, NULL); | ||
| r = H5Pget_filter_by_id( | ||
| dcpl, FILTER_BLOSC, &flags, &cd_nelmts, cd_values, 0, NULL, NULL | ||
| ); | ||
| if (r < 0) return -1; | ||
|
|
||
| if (nelements < 4) nelements = 4; /* First 4 slots reserved. */ | ||
| if (cd_nelmts < 4) cd_nelmts = 4; /* First 4 slots reserved. */ | ||
|
|
||
| /* Set Blosc info in first two slots */ | ||
| values[0] = FILTER_BLOSC_VERSION; | ||
| values[1] = BLOSC_VERSION_FORMAT; | ||
| cd_values[0] = FILTER_BLOSC_VERSION; | ||
| cd_values[1] = BLOSC_VERSION_FORMAT; | ||
|
|
||
| ndims = H5Pget_chunk(dcpl, 32, chunkdims); | ||
| if (ndims < 0) return -1; | ||
|
|
@@ -108,6 +118,7 @@ herr_t blosc_set_local(hid_t dcpl, hid_t type, hid_t space) { | |
| if (typesize == 0) return -1; | ||
| /* Get the size of the base type, even for ARRAY types */ | ||
| classt = H5Tget_class(type); | ||
| if (classt == H5T_NO_CLASS) return -1; | ||
| if (classt == H5T_ARRAY) { | ||
| /* Get the array base component */ | ||
| super_type = H5Tget_super(type); | ||
|
|
@@ -120,22 +131,25 @@ herr_t blosc_set_local(hid_t dcpl, hid_t type, hid_t space) { | |
|
|
||
| /* Limit large typesizes (they are pretty expensive to shuffle | ||
| and, in addition, Blosc does not handle typesizes larger than | ||
| 256 bytes). */ | ||
| 255 bytes). */ | ||
| if (basetypesize > BLOSC_MAX_TYPESIZE) basetypesize = 1; | ||
| values[2] = basetypesize; | ||
| cd_values[2] = basetypesize; | ||
|
|
||
| /* Get the size of the chunk */ | ||
| bufsize = typesize; | ||
| /* Get the size of the chunk. This is unused by blosc_filter(). | ||
| It is retained for backward compatibility. | ||
| */ | ||
| chunksize = typesize; | ||
| for (i = 0; i < ndims; i++) { | ||
| bufsize *= chunkdims[i]; | ||
| chunksize *= chunkdims[i]; | ||
| } | ||
| values[3] = bufsize; | ||
| cd_values[3] = chunksize; | ||
|
|
||
| #ifdef BLOSC_DEBUG | ||
| fprintf(stderr, "Blosc: Computed buffer size %d\n", bufsize); | ||
| fprintf(stderr, "Blosc: typesize=%d; chunksize=%d\n", | ||
| typesize, chunksize); | ||
| #endif | ||
|
|
||
| r = H5Pmodify_filter(dcpl, FILTER_BLOSC, flags, nelements, values); | ||
| r = H5Pmodify_filter(dcpl, FILTER_BLOSC, flags, cd_nelmts, cd_values); | ||
| if (r < 0) return -1; | ||
|
|
||
| return 1; | ||
|
|
@@ -159,9 +173,15 @@ size_t blosc_filter(unsigned flags, size_t cd_nelmts, | |
| const char* complist; | ||
| char errmsg[256]; | ||
|
|
||
| assert(cd_nelmts >= 4); | ||
| assert(cd_values[0] == FILTER_BLOSC_VERSION); | ||
| assert(cd_values[1] == BLOSC_VERSION_FORMAT); | ||
| assert(nbytes > 0); | ||
| assert(*buf_size >= nbytes); | ||
|
|
||
| /* Filter params that are always set */ | ||
| typesize = cd_values[2]; /* The datatype size */ | ||
| outbuf_size = cd_values[3]; /* Precomputed buffer guess */ | ||
| assert(typesize > 0 && typesize <= BLOSC_MAX_TYPESIZE); | ||
| /* Optional params */ | ||
| if (cd_nelmts >= 5) { | ||
| clevel = cd_values[4]; /* The compression level */ | ||
|
|
@@ -200,14 +220,14 @@ size_t blosc_filter(unsigned flags, size_t cd_nelmts, | |
| proceeds. | ||
| */ | ||
|
|
||
| outbuf_size = (*buf_size); | ||
| outbuf_size = nbytes; | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In the event that for whatever reason libhdf5 passed us a buffer that is larger than the data, this saves us some bytes. |
||
|
|
||
| #ifdef BLOSC_DEBUG | ||
| fprintf(stderr, "Blosc: Compress %zd chunk w/buffer %zd\n", | ||
| nbytes, outbuf_size); | ||
| fprintf(stderr, "Blosc: Compress %zd bytes chunk (typesize=%d)\n", | ||
| nbytes, typesize); | ||
| #endif | ||
|
|
||
| outbuf = malloc(outbuf_size); | ||
| outbuf = malloc(nbytes); | ||
|
|
||
| if (outbuf == NULL) { | ||
| PUSH_ERR("blosc_filter", H5E_CALLBACK, | ||
|
|
@@ -218,29 +238,32 @@ size_t blosc_filter(unsigned flags, size_t cd_nelmts, | |
| blosc_set_compressor(compname); | ||
| status = blosc_compress(clevel, doshuffle, typesize, nbytes, | ||
| *buf, outbuf, nbytes); | ||
| if (status == 0) goto failed; /* compressed size > input size. This is OK. */ | ||
| if (status < 0) { | ||
| /* Internal error */ | ||
| PUSH_ERR("blosc_filter", H5E_CALLBACK, "Blosc compression error"); | ||
| goto failed; | ||
| } | ||
| assert((size_t)status <= nbytes); | ||
|
|
||
| /* We're decompressing */ | ||
| } else { | ||
| /* declare dummy variables */ | ||
| size_t cbytes, blocksize; | ||
|
|
||
| free(outbuf); | ||
|
|
||
| /* Extract the exact outbuf_size from the buffer header. | ||
| * | ||
| * NOTE: the guess value got from "cd_values" corresponds to the | ||
| * uncompressed chunk size but it should not be used in a general | ||
| * cases since other filters in the pipeline can modify the buffere | ||
| * size. | ||
| * NOTE: cd_values[3] contains the uncompressed chunk size. | ||
| * It should not be used in general cases since other filters in the | ||
| * pipeline can modify the buffer size. | ||
| */ | ||
| blosc_cbuffer_sizes(*buf, &outbuf_size, &cbytes, &blocksize); | ||
| assert(cbytes == nbytes); | ||
|
|
||
| #ifdef BLOSC_DEBUG | ||
| fprintf(stderr, "Blosc: Decompress %zd chunk w/buffer %zd\n", nbytes, outbuf_size); | ||
| fprintf(stderr, | ||
| "Blosc: Decompress %zd bytes compressed chunk into %zd bytes buffer\n", | ||
| nbytes, outbuf_size); | ||
| #endif | ||
|
|
||
| outbuf = malloc(outbuf_size); | ||
|
|
@@ -254,18 +277,20 @@ size_t blosc_filter(unsigned flags, size_t cd_nelmts, | |
| if (status <= 0) { /* decompression failed */ | ||
| PUSH_ERR("blosc_filter", H5E_CALLBACK, "Blosc decompression error"); | ||
| goto failed; | ||
| } /* if !status */ | ||
| } | ||
|
|
||
| } /* compressing vs decompressing */ | ||
|
|
||
| if (status != 0) { | ||
| free(*buf); | ||
| *buf = outbuf; | ||
| *buf_size = outbuf_size; | ||
| return status; /* Size of compressed/decompressed data */ | ||
| } | ||
| assert(status > 0); | ||
| assert(status <= outbuf_size); | ||
| /* Compression successful */ | ||
| free(*buf); | ||
| *buf = outbuf; | ||
| *buf_size = outbuf_size; | ||
| return status; /* Size of compressed/decompressed data */ | ||
|
|
||
| failed: | ||
| /* Note: we will reach this when compressed size > original size. */ | ||
| free(outbuf); | ||
| return 0; | ||
|
|
||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suppresses a deprecation warning