Skip to content

Commit

Permalink
Improve the fix for Unidata#350 included in Unidata#1119
Browse files Browse the repository at this point in the history
1) We have to use H5Tequal() to compare HDF5 type IDs.
2) When checking if we can re-use an NC_CHAR attribute it is enough to
   compare data types (H5Tequal() takes care of the size comparison).
3) This commit adds missing code (reuse_att was set but not used).

Now an attribute in a NetCDF-4 file can be modified as many times as
necessary, as long its type and length remain the same.

Modifications changing either type or length of an attribute require
deleting and re-creating an attribute which increments the attribute
order creation index. Once this index reaches 65535 all attribute
modifications (for a particular group or variable) will fail.

For reference:

Issue 350 title: NetCDF-4 limits the number of times an attribute can
be modified

Pull request 1119 title: Fix checking for HDF5 max dims, no longer
re-create atts if not needed, confirm behavior for HDF5 cyclical
files, allow user to set mpiexec
  • Loading branch information
ckhroulev committed Nov 13, 2019
1 parent 108e938 commit dd181de
Showing 1 changed file with 39 additions and 12 deletions.
51 changes: 39 additions & 12 deletions libhdf5/nc4hdf.c
Original file line number Diff line number Diff line change
Expand Up @@ -512,7 +512,6 @@ put_att_grpa(NC_GRP_INFO_T *grp, int varid, NC_ATT_INFO_T *att)
hid_t existing_att_typeid = 0, existing_attid = 0, existing_spaceid = 0;
hsize_t dims[1]; /* netcdf attributes always 1-D. */
htri_t attr_exists;
int reuse_att = 0; /* Will be true if we can re-use an existing att. */
void *data;
int phoney_data = 99;
int retval = NC_NOERR;
Expand Down Expand Up @@ -612,31 +611,59 @@ put_att_grpa(NC_GRP_INFO_T *grp, int varid, NC_ATT_INFO_T *att)
BAIL(NC_EATTMETA);

/* How big is the attribute? */
if ((existing_spaceid = H5Aget_space(existing_attid)) < 0)
if (att->nc_typeid == NC_CHAR) {
/* For text attributes the size is specified in the datatype
and it is enough to compare types using H5Tequal(). Here
we set npoints to disable the second part of the
comparison below. */
npoints = att->len;
}
else
{
if ((existing_spaceid = H5Aget_space(existing_attid)) < 0)
BAIL(NC_EATTMETA);
if ((npoints = H5Sget_simple_extent_npoints(existing_spaceid)) < 0)
if ((npoints = H5Sget_simple_extent_npoints(existing_spaceid)) < 0)
BAIL(NC_EATTMETA);
}

/* Delete the attribute. */
if (file_typeid != existing_att_typeid || npoints != att->len)
if (!H5Tequal(file_typeid, existing_att_typeid) || npoints != att->len)
{
/* The attribute exists but we cannot re-use it. */

/* Delete the attribute. */
if (H5Adelete(locid, att->hdr.name) < 0)
BAIL(NC_EHDFERR);

/* Re-create the attribute with the type and length
reflecting the new value (or values). */
if ((attid = H5Acreate(locid, att->hdr.name, file_typeid, spaceid,
H5P_DEFAULT)) < 0)
BAIL(NC_EATTMETA);

/* Write the values, (even if length is zero). */
if (H5Awrite(attid, file_typeid, data) < 0)
BAIL(NC_EATTMETA);
}
else
{
reuse_att++;
/* The attribute exists and we can re-use it. */

/* Write the values, re-using the existing attribute. */
if (H5Awrite(existing_attid, file_typeid, data) < 0)
BAIL(NC_EATTMETA);
}
}
} else {
/* The attribute does not exist yet. */

/* Create the attribute. */
if ((attid = H5Acreate(locid, att->hdr.name, file_typeid, spaceid,
H5P_DEFAULT)) < 0)
/* Create the attribute. */
if ((attid = H5Acreate(locid, att->hdr.name, file_typeid, spaceid,
H5P_DEFAULT)) < 0)
BAIL(NC_EATTMETA);

/* Write the values, (even if length is zero). */
if (H5Awrite(attid, file_typeid, data) < 0)
/* Write the values, (even if length is zero). */
if (H5Awrite(attid, file_typeid, data) < 0)
BAIL(NC_EATTMETA);
}

exit:
if (file_typeid && H5Tclose(file_typeid))
Expand Down

0 comments on commit dd181de

Please sign in to comment.