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

Initialize members of vldata to make sure that nc_free_vlens() succeeds. See #527. #605

Merged
merged 5 commits into from
Nov 28, 2016

Conversation

ckhroulev
Copy link
Contributor

Note that free(NULL) is a no-op.

@jswhit
Copy link
Collaborator

jswhit commented Nov 16, 2016

Could @dopplershift 's test from issue #527 be added to tst_vlen.py along with a Changelog entry?

@ckhroulev
Copy link
Contributor Author

@jswhit Absolutely!

@ckhroulev
Copy link
Contributor Author

Note that Travis CI uses NetCDF 4.1.1 while AppVeyor has 4.3.3.1.

I'll post an update once I have more info. (I'll have to install NetCDF and HDF5 versions matching ones used by Travis CI scripts.)

@jswhit
Copy link
Collaborator

jswhit commented Nov 17, 2016

4.3 can be installed in travis ci trusty

https://launchpad.net/~heiko-klein/+archive/ubuntu/netcdf

@jswhit
Copy link
Collaborator

jswhit commented Nov 18, 2016

The travis tests still fail even with hdf5-1.8.15-patch1 and necdf-4.4.0 (see pull request #606).

Works fine for me on OSX. Maybe it's related to the compiler toolchain?

@jswhit
Copy link
Collaborator

jswhit commented Nov 21, 2016

I'd like to go ahead and merge this, since it is the right thing to do. Let's just disable the test for right now until we figure out why it fails on Ubuntu.

@ckhroulev
Copy link
Contributor Author

@jswhit If I understand this correctly, the test I added will fail with NetCDF versions < 4.4.1 (see Unidata/netcdf-c#221). I have no idea why it passed on AppVeyor (I blame it on the undefined behavior of memcpy when one of the pointer arguments is null).

@jswhit
Copy link
Collaborator

jswhit commented Nov 24, 2016

Let's wrap the test with if __netcdf4libversion__ > '4.4.0' then.

@ckhroulev
Copy link
Contributor Author

@jswhit Regarding turning the test off for NetCDF < 4.4.1: I did this already (please see the code).

(One of AppVeyor checks failed during "git clone". The rest passed, skipping this test, as expected.)

@jswhit jswhit merged commit 1be10e8 into Unidata:master Nov 28, 2016
@jswhit
Copy link
Collaborator

jswhit commented Nov 28, 2016

OK, great. Merging now then.

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