Skip to content

BUG: Increase buffer used to store filepath #595

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

Merged
merged 3 commits into from
Oct 26, 2016

Conversation

jjhelmus
Copy link
Contributor

Increase the buffer used to store the filepath by one character to accommodate
the NULL character copied by the nc_inq_path function.

closes #585

Increase the buffer used to store the filepath by one character to accommodate
the NULL character copied by the nc_inq_path function.

closes Unidata#585
@jjhelmus
Copy link
Contributor Author

Note sure if it is typical to regenerate the _netCDF4.c file in the pull request or not. Happy to if it is desired.

@dopplershift
Copy link
Member

Awesome, thanks for running this down!

@jswhit
Copy link
Collaborator

jswhit commented Oct 24, 2016

Please add a Changelog entry and regenerate _netCDF.c with all values set to zero in include/constants.pyx

@jjhelmus
Copy link
Contributor Author

Added to the Changelog and regenerated _netCDF.c. Not sure how useful the latter is given that the change is excluded from that file.

@jswhit
Copy link
Collaborator

jswhit commented Oct 25, 2016

The change will be included in _netCDF.c, if you regenerated it from the updated netCDF4.pyx.

@jjhelmus
Copy link
Contributor Author

The change to _netCDF.pyx made in this PR are inside the IF HAS_NC_INQ_PATH block and therefore, unless I am misunderstanding the instructions, are excluded from the _netCDF.c file when it is generated with a include/constants.pyx file with all values set to zero.

Specifically the block in question is as follows:

    def filepath(self):
        """
**`filepath(self)`**

Get the file system path (or the opendap URL) which was used to
open/create the Dataset. Requires netcdf >= 4.1.2"""
        cdef int ierr
        cdef size_t pathlen
        cdef char *c_path
        IF HAS_NC_INQ_PATH:
            with nogil:
                ierr = nc_inq_path(self._grpid, &pathlen, NULL)
            if ierr != NC_NOERR:
                raise RuntimeError((<char *>nc_strerror(ierr)).decode('ascii'))
            c_path = <char *>malloc(sizeof(char) * (pathlen + 1))
            if not c_path:
                raise MemoryError()
            try:
                with nogil:
                    ierr = nc_inq_path(self._grpid, &pathlen, c_path)
                if ierr != NC_NOERR:
                    raise RuntimeError((<char *>nc_strerror(ierr)).decode('ascii'))
                py_path = c_path[:pathlen] # makes a copy of pathlen bytes from c_string
            finally:
                free(c_path)
            return py_path.decode('ascii')
        ELSE:
            msg = """
filepath method not enabled.  To enable, install Cython, make sure you have
version 4.1.2 or higher of the netcdf C lib, and rebuild netcdf4-python."""
            raise ValueError(msg)

@jswhit jswhit merged commit b3f12f8 into Unidata:master Oct 26, 2016
@jswhit
Copy link
Collaborator

jswhit commented Oct 26, 2016

Yes, you are correct. The _netCDF4.c file should be unchanged. Going ahead with the merge now...

Thanks again!

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.

Possible memory leak
3 participants