Hi Richard,
Thanks for confirming this. In my local copy I also fixed function
nf_def_dim in the same file (nf_dim.f90).
As an aside, some of the 'null'/'denull' code in that file could be
simplified a bit to avoid temporary strings. When creating a null
terminated string to pass to a C routine, all one really needs is, for
example instead of:
Character(LEN=(LEN(name)+1)) :: cname
...
cname = addCNullChar(name, ie)
...
cstatus = nc_inq_dimid(cncid, cname(1:ie+1), cdimid)
one could simply write:
cstatus = nc_inq_dimid(cncid, trim (name) // iachar (0), cdimid)
And when accepting a potentially null terminated string return value
from a C routine, instead of:
Character(LEN=(LEN(name)+1)) :: cname
...
tmpname = REPEAT(" ", LEN(tmpname))
name = REPEAT(" ", LEN(name))
nlen = LEN(name)
! Get tmpname and cdlen from C interface
cstatus = nc_inq_dim(cncid, cdimid, tmpname, cdlen)
! Strip C null char from tmpname if present and set end of string
name = stripCNullChar(tmpname, nlen)
one could write:
! Get tmpname and cdlen from C interface
name = " "
cstatus = nc_inq_dim(cncid, cdimid, name, cdlen)
null_pos = INDEX (name, iachar (0))
if (null_pos > 0) then
name(null_pos:) = " "
end if
Actually, in light of the original issue, this last piece of code might
be better written:
cstatus = nc_inq_dim(cncid, cdimid, name, cdlen)
if (cstatus == NC_NOERR) then
null_pos = INDEX (name, iachar (0))
if (null_pos > 0) then
name(null_pos:) = " "
end if
else
name = " "
end if
Walter
On 05/22/2014 06:58 AM, Richard Weed wrote:
Walter
Thanks for pointing this out. Its the first time in the 8+ years since
I wrote the original version of these functions that anyone has caught this
bug. The most appropriate (quick) fix is to change
If (cdimid == -1)
to
If (cstatus == NC_EBADDIM)
or
if (cstatus /= NC_NOERR) as you suggest.
Also, initializing cdimid to something is a good idea since it would
not be passed into nc_inq_dimid due to the Intent(OUT) restriction
I have on the associated dummy argument
Also, FYI - It appears the underlying C functions never return a value
of -1.
This was only done in the cfortran.h generated wrapper routines which I
used to develop the C interoperability interfaces. There are some other
potential issues that need to be addressed long term. The C functions allow
you to call NC_inq_dimid and NC_def_dimid with the dimid pointer in the
interface set to a NULL. In this case all that's returned are the status
values (no value of dimid). I can emulate this in the new Fortran code but
I personally don't see much value in doing it.
RW
On 05/21/2014 02:31 PM, W Spector wrote:
Dear NetCDFers,
Using the NetCDF Fortran API v4.4.0rc1. When running under valgrind,
my unit
tests will sometimes show the following:
==26685== Thread 1:
==26685== Conditional jump or move depends on uninitialised value(s)
==26685== at 0x72FC83E: nf_inq_dimid_ (nf_dim.f90:136)
==26685== by 0x736B13F: __netcdf_MOD_nf90_inq_dimid
(netcdf_dims.f90:21)
==26685== by 0x6181E82: __esmfnf_mod_MOD_pio_inq_dimid
(nf_mod.F90:1218)
If one looks around line 136 in nf_dim.f90, the issue is that I have
unit tests where the eventual call to nc_inq_dimid is made with a bad
ncid. This results in a return code /= NC_NOERR. Then, at line 136,
undefined stack trash from cdimid is used in the if test.
There are a few ways this could be fixed, including:
1.) cdimid could be initialized to something before the call to
nc_inq_dimid is made,
2.) over on the C side, nc_inq_dimid could return a reliable value for
cdimid when an error occurs,
3.) nf_inq_dimid could check for NC_NOERR before attempting to use cdimid
Other procedures in the nf_dim.f90 file have the same issue. And of
course other files should also be checked.
Walter
_______________________________________________
netcdfgroup mailing list
netcdfgroup@xxxxxxxxxxxxxxxx
For list information or to unsubscribe, visit:
http://www.unidata.ucar.edu/mailing_lists/
