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

nc_delete_mp() uses NC3_DATA macro, but is not a dispatch table function #926

Closed
edhartnett opened this issue Apr 17, 2018 · 1 comment
Closed

Comments

@edhartnett
Copy link
Contributor

Here's something I ran into while doing the HDF5/libsrc4 conversion. I also have the easy fix.

In libsrc/nc3internal.c we have:

/**************************************************/
int
nc_delete_mp(const char * path, int basepe)
{
	NC *nc;
	NC3_INFO* nc3;
	int status;
	int ncid;
	size_t chunk = 512;

	status = nc_open(path,NC_NOWRITE,&ncid);
        if(status) return status;

	status = NC_check_id(ncid,&nc);
        if(status) return status;
	nc3 = NC3_DATA(nc);

	nc3->chunk = chunk;

#if defined(LOCKNUMREC) /* && _CRAYMPP */
	if (status = NC_init_pe(nc3, basepe)) {
		return status;
	}
#else
	/*
	 * !_CRAYMPP, only pe 0 is valid
	 */
	if(basepe != 0)
		return NC_EINVAL;
#endif

	(void) nc_close(ncid);
	if(unlink(path) == -1) {
	    return NC_EIO;	/* No more specific error code is appropriate */
	}
	return NC_NOERR;
}

Note that we are applying NC3_DATA macro, and then modifying the "chunk" field within it.

Problem is, this is not a dispatch function. ;-) So it is also called by nc_test when testing netCDF-4 files. The setting of nc3->chunk changes some random part of the netcdf-4 structure, which happens to have no effect (or didn't, until I did the HDF5/libsrc4 conversion, and encountered this error).

The fix is simple - there is no reason to be messing with nc3->chunk, as the file is about to be deleted. I simply removed the two lines of code.

	nc3 = NC3_DATA(nc);
	nc3->chunk = chunk;

The fix for this will be part of my PR for #856. I wanted to high-light this change since it is in the classic code, outside the libsrc4 directory where all the rest of the #856 changes will be found.

@edhartnett
Copy link
Contributor Author

This fix made it into one of the aggregate PRs and is now in the code.

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 a pull request may close this issue.

1 participant