Skip to content
This repository has been archived by the owner on Sep 1, 2022. It is now read-only.

Fixes #445: GRIB1 data length overflow #1005

Merged

Conversation

cofinoa
Copy link
Contributor

@cofinoa cofinoa commented Jan 12, 2018

Due to a trick done by ECMWF's GRIBEX to support large GRIBs, we need a special treatment to fix the length of the GRIB message. See:
https://software.ecmwf.int/wiki/display/EMOS/Changes+in+cycle+000281
#445

@cofinoa
Copy link
Contributor Author

cofinoa commented Jan 12, 2018

I'm not sure if I have made the PR on the correct branch.
The commit has been made for the the tag 4.6.11.

@lesserwhirls
Copy link
Collaborator

lesserwhirls commented Jan 12, 2018

Nice - thank you for the contribution, @cofinoa. Would you happen to have a record or two that we could use for tests? This is something that we'd want a test, for sure.

edit: I see @sefadolunay provided a few sample grids on that issue, so we're good there.

@lesserwhirls
Copy link
Collaborator

I'm currently running this PR on our Jenkins instance to see how our full tests turn out.

@cofinoa
Copy link
Contributor Author

cofinoa commented Jan 12, 2018

I have extracted one GRIB message from @sefadolunay sample file:
https://meteo.unican.es/droppings/antonio/ecmwfGRIB1large.grib

@lesserwhirls
Copy link
Collaborator

Our test suite on Jenkins passed without any impact on our GRIB tests, so that's good. It would be good to get this on 5.0 as well, and I think the changes should go on cleanly. I'll add some tests for this on 5.0 once we get it applied there. @cofinoa - would you mind rebasing your PR to pick up the latests changes in master?

@cofinoa cofinoa force-pushed the #445_GRIB1_data_length_overflow branch from 028a22a to 505f270 Compare January 12, 2018 22:07
@cofinoa
Copy link
Contributor Author

cofinoa commented Jan 12, 2018

@lesserwhirls

I have rebased my PR to the latest changes at master ... or at least I think I have made ...

@lesserwhirls lesserwhirls merged commit 9fc0776 into Unidata:master Jan 16, 2018
@sefadolunay
Copy link

Here I provide more test files: test1 test2

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants