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

added merged HDF4 changes #922

Merged

Conversation

edhartnett
Copy link
Contributor

@edhartnett edhartnett commented Apr 4, 2018

This PR accumulates changes with the HDF4 dispatch code. Changes include:

  • Moved some functions to libdispatch which can be used in more than one dispatch layer. Use these shared functions in the pnetcdf layer, which previously re-implemented them. The functions are also used in the external dispatch later, the AB-dispatch package.

  • Use of void * in INFO_T structs to hold HDF4 data. Now libsrc4 does not need to include hdf4 header files. This will also be done for HDF5 as part of Separate internal netCDF-4 data model from HDF5 code #856.

  • HDF4 did not used to handle data conversion correctly (see HDF4 reads do not convert type as they should #916). This PR fixes that.

  • As requested, all non-static functions now start with "nc" or "NC".

  • Long function in hdf4file.c has been broken into several functions, reducing some code repetition.

  • Extra test added to increase test coverage and test some previously untested features (like data conversion).

Fixes #916.
Fixes #913.
Fixes #871.
Fixes #870.
Part of #856.
Part of #177.

Once this is past review and merged, I'll present a PR with separation of HDF5 from libsrc4 (#856).

@DennisHeimbigner
Copy link
Collaborator

DennisHeimbigner commented Apr 5, 2018

[nevermind, I see you just moved the code to much further down]
If you are using the libsrc4 code to create the NC_HDF5_FILE_INFO_T
object, then you need to keep this code:

nclistfree(h5->alldims);
nclistfree(h5->allgroups);
nclistfree(h5->alltypes);

otherwise you will have a memory leak.
You better check to make sure.

Copy link
Collaborator

@DennisHeimbigner DennisHeimbigner left a comment

Choose a reason for hiding this comment

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

Did a quick scan and it looks good to me.

@edhartnett
Copy link
Contributor Author

edhartnett commented Apr 5, 2018 via email

@edhartnett
Copy link
Contributor Author

@WardF and we get my open PRs merged soon? I would like to present my PR for #856, but I need my outstanding PRs to be merged first. Thanks!

@WardF WardF merged commit 9ad46d7 into Unidata:master Apr 16, 2018
@edhartnett
Copy link
Contributor Author

Thanks!! ;-)

@edhartnett edhartnett deleted the ejh_hdf4_again branch May 14, 2018 14:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants