From 6a06eaba15463330e0c96d0d99ad3dc4bc4565b2 Mon Sep 17 00:00:00 2001
From: Dennis Heimbigner <dennis.heimbigner@gmail.com>
Date: Sun, 16 Mar 2025 18:13:51 -0600
Subject: [PATCH] Fix bug in copying compound types with variable length
fields.
re: Issue https://github.com/Unidata/netcdf-c/issues/3110
There was a two-line typo in the code in netcdf-c/libdispatch/distance_intern.c.
This PR fixes it and adds a test case in netcdf-c/unit_test/tst_reclaim.c.
---
libdispatch/dinstance.c | 1 +
libdispatch/dinstance_intern.c | 4 +-
unit_test/Makefile.am | 4 +-
unit_test/reclaim_tests.baseline | 94 +++++++++++++++++++
unit_test/reclaim_tests.cdl | 14 +++
unit_test/run_reclaim_tests.sh | 4 +
unit_test/tst_reclaim.c | 155 ++++++++++++++++++++++++++++---
7 files changed, 258 insertions(+), 18 deletions(-)
create mode 100644 unit_test/reclaim_tests.baseline
diff --git a/libdispatch/dinstance.c b/libdispatch/dinstance.c
index cef9b2e89..fbb2db714 100644
--- a/libdispatch/dinstance.c
+++ b/libdispatch/dinstance.c
@@ -284,6 +284,7 @@ done:
@param xtype type id
@param memory ptr to top-level memory to dump
@param count number of instances of the type in memory block
+@param bufp return ptr to a buffer of dump'd data
@return error code
*/
diff --git a/libdispatch/dinstance_intern.c b/libdispatch/dinstance_intern.c
index f1dac8eda..894ce40e2 100644
--- a/libdispatch/dinstance_intern.c
+++ b/libdispatch/dinstance_intern.c
@@ -439,8 +439,8 @@ copy_datar(NC_FILE_INFO_T* file, NC_TYPE_INFO_T* utype, Position src, Position d
/* optimize: string field type */
if(field->nc_typeid == NC_STRING) {
- char** srcstrvec = (char**)src.memory;
- char** dststrvec = (char**)dst.memory;
+ char** srcstrvec = (char**)fsrc.memory;
+ char** dststrvec = (char**)fdst.memory;
for(i=0;i<arraycount;i++)
{if(srcstrvec[i] != NULL) {dststrvec[i] = strdup(srcstrvec[i]);} else {dststrvec[i] = NULL;}}
continue; /* move to next field */
diff --git a/unit_test/Makefile.am b/unit_test/Makefile.am
index e1ae642f1..a2f86e84d 100644
--- a/unit_test/Makefile.am
+++ b/unit_test/Makefile.am
@@ -70,8 +70,8 @@ EXTRA_DIST += nctest_netcdf4_classic.nc reclaim_tests.cdl
EXTRA_DIST += ref_get.txt ref_set.txt
EXTRA_DIST += ref_xget.txt ref_xset.txt
EXTRA_DIST += ref_provparse.txt
-
-CLEANFILES = reclaim_tests*.txt reclaim_tests.nc tmp_*.txt
+EXTRA_DIST += reclaim_tests.baseline
+CLEANFILES = reclaim_tests*.txt reclaim_tests.nc reclaim_tests.dmp tmp_*.txt
# If valgrind is present, add valgrind targets.
@VALGRIND_CHECK_RULES@
diff --git a/unit_test/reclaim_tests.baseline b/unit_test/reclaim_tests.baseline
new file mode 100644
index 000000000..92c5d2891
--- /dev/null
+++ b/unit_test/reclaim_tests.baseline
@@ -0,0 +1,94 @@
+netcdf reclaim_tests {
+types:
+ int(*) vint_t ;
+ string(*) vstr_t ;
+ compound cmpd_atomic_t {
+ int f1 ;
+ float f2(2, 2) ;
+ }; // cmpd_atomic_t
+ compound cmpd_str_t {
+ string f1(3) ;
+ }; // cmpd_str_t
+ compound cmpd_str_att_t {
+ string field0 ;
+ int field1 ;
+ string fields2 ;
+ }; // cmpd_str_att_t
+ vint_t(*) vvint_t ;
+ vstr_t(*) vvstr_t ;
+ compound cmpd_vlen_t {
+ vint_t f1(2) ;
+ }; // cmpd_vlen_t
+ compound cmpd_cmpd_t {
+ vstr_t f1(2) ;
+ cmpd_vlen_t f2(2) ;
+ }; // cmpd_cmpd_t
+ cmpd_atomic_t(*) vcmpd_atomic_t ;
+ cmpd_str_t(*) vcmpd_str_t ;
+ cmpd_vlen_t(*) vcmpd_vlen_t ;
+ cmpd_cmpd_t(*) vcmpd_cmpd_t ;
+dimensions:
+ d1 = 1 ;
+ d2 = 2 ;
+ d3 = 3 ;
+ d4 = 4 ;
+variables:
+ int intvar(d4) ;
+ string strvar(d4) ;
+ vint_t vintvar(d4) ;
+ vstr_t vstrvar(d4) ;
+ vvint_t vvintvar(d4) ;
+ vvstr_t vvstrvar(d4) ;
+ cmpd_atomic_t catomvar(d2) ;
+ cmpd_str_t cstrvar(d2) ;
+ cmpd_vlen_t cvlenvar(d2) ;
+ cmpd_cmpd_t ccmpdvar(d2) ;
+ vcmpd_atomic_t vcatomvar ;
+ vcmpd_str_t vcstrvar(d2) ;
+ vcmpd_vlen_t vcvlenvar(d3) ;
+ vcmpd_cmpd_t vccmpdvar ;
+ int int3110 ;
+ cmpd_str_att_t int3110:stratt = {"field0", 31, "field3"} ;
+data:
+
+ intvar = 17, 13, 11, 7 ;
+
+ strvar = "a", "ab", "abc", "abcd" ;
+
+ vintvar = {1}, {1, 2}, {1, 2, 3}, {1, 2, 3, 4} ;
+
+ vstrvar = {"a", "ab", "abc", "abcd"}, {"abcd", "abc", "ab"}, {"a", "ab"},
+ {"abcd"} ;
+
+ vvintvar = {{1}, {1, 2}}, {{1, 2, 3}}, {{1, 2, 3, 4}},
+ {{17, 13}, {11, 7}, {5, 3}, {2, 1}} ;
+
+ vvstrvar = {{"1"}, {"1", "2"}}, {{"1", "2", "3"}}, {{"1", "2", "3", "4"}},
+ {{"17", "13"}, {"11", "7"}, {"5", "3"}, {"2", "1"}} ;
+
+ catomvar = {17, {1.1, 2.2, 3.3, 4.4}}, {27, {4.1, 3.2, 3.3, 1.4}} ;
+
+ cstrvar = {{"a", "abc", "abcd"}}, {{"x", "yz", "ijkl"}} ;
+
+ cvlenvar = {{{17, 13}, {11}}}, {{{3, 5}, {7, 11}}} ;
+
+ ccmpdvar =
+ {{{"xxx"}, {"yyy"}}, {{{{17, 13}, {12, 9}}}, {{{1, 3}, {2, 11}}}}},
+ {{{"uv"}, {"w"}}, {{{{117, 113}, {112, 119}}}, {{{111, 113}, {112, 1111}}}}} ;
+
+ vcatomvar = {{17, {1.1, 2.2, 3.3, 4.4}}, {27, {4.1, 3.2, 3.3, 1.4}}} ;
+
+ vcstrvar =
+ {{{"c11a", "c12a", "c13a"}}, {{"c11b", "c12b", "c13b"}}, {{"c11c", "c12c", "c13c"}}, {{"c11d", "c12d", "c13d"}}},
+ {{{"c21a", "c22a", "c23a"}}, {{"c21b", "c22b", "c23b"}}, {{"c21c", "c22c", "c23c"}}, {{"c21d", "c22d", "c23d"}}} ;
+
+ vcvlenvar =
+ {{{{117, 113}, {111}}}, {{{13, 15}, {17, 111}}}, {{{217, 213}, {211}}}, {{{23, 25}, {27, 211}}}},
+ {{{{2117, 2113}, {2211}}}, {{{2223, 2215}, {2227, 22111}}}, {{{22217, 22213}, {22211}}}, {{{2223, 2225}, {2227, 22211}}}},
+ {{{{33117, 33113}, {33111}}}, {{{3313, 3315}, {3317, 33111}}}, {{{33217, 33213}, {33211}}}, {{{3323, 3325}, {3327, 33211}}}} ;
+
+ vccmpdvar =
+ {{{{"xxx"}, {"yyy"}}, {{{{17, 13}, {12, 9}}}, {{{1, 3}, {2, 11}}}}}, {{{"uv"}, {"w"}}, {{{{117, 113}, {112, 119}}}, {{{111, 113}, {112, 1111}}}}}} ;
+
+ int3110 = 117 ;
+}
diff --git a/unit_test/reclaim_tests.cdl b/unit_test/reclaim_tests.cdl
index acf716793..352cec52f 100644
--- a/unit_test/reclaim_tests.cdl
+++ b/unit_test/reclaim_tests.cdl
@@ -19,6 +19,13 @@ types:
vstr_t f1(2) ;
cmpd_vlen_t f2(2);
};
+ /* Specific test types */
+ /* ref: Issue https://github.com/Unidata/netcdf-c/issues/3110 */
+ compound cmpd_str_att_t { /* compound with separate string typed field(s) */
+ string field0 ;
+ int field1 ;
+ string fields2 ;
+ };
cmpd_atomic_t(*) vcmpd_atomic_t;
cmpd_str_t(*) vcmpd_str_t;
@@ -54,6 +61,10 @@ variables:
vcmpd_vlen_t vcvlenvar(d3);
vcmpd_cmpd_t vccmpdvar;
+ /* Special cases */
+ int int3110;
+ cmpd_str_att_t int3110:stratt = {"field0", 31, "field3"} ;
+
data:
intvar = 17, 13, 11, 7 ;
strvar = "a", "ab", "abc", "abcd" ;
@@ -70,6 +81,9 @@ data:
vcvlenvar = {{{{117,113},{111}}}, {{{13,15},{17,111}}},{{{217,213},{211}}}, {{{23,25},{27,211}}}}, {{{{2117,2113},{2211}}}, {{{2223,2215},{2227,22111}}},{{{22217,22213},{22211}}}, {{{2223,2225},{2227,22211}}}}, {{{{33117,33113},{33111}}}, {{{3313,3315},{3317,33111}}},{{{33217,33213},{33211}}}, {{{3323,3325},{3327,33211}}}};
vccmpdvar = {{{{"xxx"},{"yyy"}},{{{{17,13},{12,9}}},{{{1,3},{2,11}}}}}, {{{"uv"},{"w"}},{{{{117,113},{112,119}}},{{{111,113},{112,1111}}}}}};
+ /* Special cases */
+ int3110 = 117;
+
} //reclaim_tests
diff --git a/unit_test/run_reclaim_tests.sh b/unit_test/run_reclaim_tests.sh
index d17777cf7..b6e2cdd26 100755
--- a/unit_test/run_reclaim_tests.sh
+++ b/unit_test/run_reclaim_tests.sh
@@ -11,6 +11,10 @@ ${execdir}/tst_reclaim > reclaim_tests.txt
sed -e '/^(o)/p' -ed reclaim_tests.txt | sed -e 's/^(o) //' > reclaim_tests_o.txt
sed -e '/^(c)/p' -ed reclaim_tests.txt | sed -e 's/^(c) //' > reclaim_tests_c.txt
diff reclaim_tests_o.txt reclaim_tests_c.txt
+# Also test using ncdump
+${NCDUMP} reclaim_tests.nc > reclaim_tests.dmp
+diff reclaim_tests.dmp ${srcdir}/reclaim_tests.baseline
+
diff --git a/unit_test/tst_reclaim.c b/unit_test/tst_reclaim.c
index 869f3c71f..dbe0d8040 100644
--- a/unit_test/tst_reclaim.c
+++ b/unit_test/tst_reclaim.c
@@ -14,7 +14,6 @@ nc_dump_data => NC_dump_data
See the file "reclaim_tests.cdl" to see the input file semantics.
*/
-
#include "config.h"
#include <stdlib.h>
#include <stdio.h>
@@ -22,6 +21,7 @@ See the file "reclaim_tests.cdl" to see the input file semantics.
#include <assert.h>
#include "netcdf.h"
#include "netcdf_aux.h"
+#include "ncbytes.h"
#define NCCATCH
#include "nclog.h"
@@ -40,6 +40,8 @@ See the file "reclaim_tests.cdl" to see the input file semantics.
#define MAXOBJECTS 1024
+#define MAXATTTXT 2048
+
struct Options {
int debug;
} dumpoptions;
@@ -56,12 +58,21 @@ struct Dim {
size_t size;
};
+struct Att {
+ char name[NC_MAX_NAME];
+ int atype;
+ size_t len;
+ void* data;
+};
+
struct Var {
char name[NC_MAX_NAME];
int vid;
int tid;
size_t dimprod;
void* data;
+ int natts;
+ struct Att* atts;
};
struct Metadata {
@@ -79,6 +90,7 @@ struct Metadata {
struct Type vcmpd_str_t;
struct Type vcmpd_vlen_t;
struct Type vcmpd_cmpd_t;
+ struct Type cmpd_str_att_t;
/* dim ids */
struct Dim d1;
struct Dim d2;
@@ -98,6 +110,8 @@ struct Metadata {
struct Var vcstrvar;
struct Var vcvlenvar;
struct Var vccmpdvar;
+ /* Specials */
+ struct Var int3110;
} metadata;
/* atomic Types */
@@ -172,6 +186,29 @@ setupdim(int ncid, const char* name, struct Dim* dim)
return NCTHROW(stat);
}
+static int
+setupatts(int ncid, struct Var* var)
+{
+ int stat = NC_NOERR;
+ int i;
+ if((stat=nc_inq_varnatts(ncid,var->vid,&var->natts))) return NCTHROW(stat);
+ var->atts = (struct Att*)calloc(sizeof(struct Att),MAXOBJECTS);
+ if(var->atts == NULL) return NCTHROW(NC_ENOMEM);
+
+ for(i=0;i<var->natts;i++) {
+ struct Att* att = &var->atts[i];
+ struct Type* atype = NULL;
+ if((stat=nc_inq_attname(ncid,var->vid,i,att->name))) return NCTHROW(stat);
+ if((stat=nc_inq_att(ncid,var->vid,att->name,&att->atype,&att->len))) return NCTHROW(stat);
+ if((atype = typemap[att->atype])==NULL) return NCTHROW(NC_ENOTATT);
+ /* compute the space for the attribute */
+ if((att->data = calloc(atype->size,att->len))==NULL) return NCTHROW(NC_ENOMEM);
+ /* Get the data */
+ if((stat=nc_get_att(ncid,var->vid,att->name,att->data))) return NCTHROW(stat);
+ }
+ return NCTHROW(stat);
+}
+
static int
setupvar(int ncid, const char* name, struct Var* var)
{
@@ -190,6 +227,7 @@ setupvar(int ncid, const char* name, struct Var* var)
product *= dimsizes[i];
}
var->dimprod = product;
+ if((stat=setupatts(ncid,var))) return NCTHROW(stat);
varmap[var->vid] = var;
return NCTHROW(stat);
}
@@ -233,6 +271,10 @@ setup(struct Metadata* md)
CHECK(setupvar(md->ncid,"vcvlenvar",&md->vcvlenvar));
CHECK(setupvar(md->ncid,"vccmpdvar",&md->vccmpdvar));
+ /* Specials */
+ CHECK(setuptype(md->ncid,"cmpd_str_att_t",&md->cmpd_str_att_t));
+ CHECK(setupvar(md->ncid,"int3110",&md->int3110)); /* will also setup associated attrs */
+
done:
assert(stat == NC_NOERR);
}
@@ -251,10 +293,50 @@ readvar(int ncid, struct Var* v)
}
static int
-dumpvar(int ncid, struct Var* v, void* data, char** bufp)
+dumpvar(int ncid, struct Var* v, void* data, NCbytes* buf)
{
int stat = NC_NOERR;
- if((stat=ncaux_dump_data(ncid,v->tid,data,v->dimprod,bufp))) return NCTHROW(stat);
+ char* tmpbuf = NULL;
+ if((stat=ncaux_dump_data(ncid,v->tid,data,v->dimprod,&tmpbuf))) return NCTHROW(stat);
+ ncbytescat(buf,tmpbuf);
+ if(tmpbuf) free(tmpbuf);
+ return NCTHROW(stat);
+}
+
+static int
+dumpatt(int ncid, struct Var* v, struct Att* a, NCbytes* attbuf)
+{
+ int stat = NC_NOERR;
+ char* tmpbuf = NULL;
+ (void)v; /* unused */
+ if((stat=ncaux_dump_data(ncid,a->atype,a->data,a->len,&tmpbuf))) return NCTHROW(stat);
+ ncbytescat(attbuf,tmpbuf);
+ if(tmpbuf) free(tmpbuf);
+ return NCTHROW(stat);
+}
+
+static int
+testatts(int ncid, struct Var* v, NCbytes* attbuf)
+{
+ int stat = NC_NOERR;
+ NCbytes* databuf = ncbytesnew();
+ /* Print any attributes */
+ if(v->natts > 0) {
+ int i;
+ for(i=0;i<v->natts;i++) {
+ struct Att* att = &v->atts[i];
+ struct Type* atype = typemap[att->atype];
+ ncbytescat(attbuf,"(o)\t");
+ ncbytescat(attbuf,atype->name);
+ ncbytescat(attbuf,att->name);
+ ncbytescat(attbuf," = {");
+ ncbytesclear(databuf);
+ if((stat=dumpatt(ncid, v, att, databuf))) return NCTHROW(stat);
+ ncbytescat(attbuf,ncbytescontents(databuf));
+ ncbytescat(attbuf,"}\n");
+ }
+ }
+ ncbytesfree(databuf);
return NCTHROW(stat);
}
@@ -262,30 +344,73 @@ static int
testvar(int ncid, struct Var* v)
{
int stat = NC_NOERR;
+ NCbytes* tmpbuf = ncbytesnew();
+ NCbytes* origbuf = ncbytesnew();
+ NCbytes* copybuf = ncbytesnew();
char* buforig = NULL;
char* bufcopy = NULL;
+ char* attbuforig = NULL;
+ char* attbufcopy = NULL;
void* copy = NULL;
-
+ /* Test original */
if((stat=readvar(ncid,v))) return NCTHROW(stat);
- if((stat=dumpvar(ncid,v,v->data,&buforig))) return NCTHROW(stat);
- printf("(o) %s: %s\n",v->name,buforig);
- // Test copying
+ if((stat=dumpvar(ncid,v,v->data,tmpbuf))) return NCTHROW(stat);
+ ncbytescat(origbuf,"(o) ");
+ ncbytescat(origbuf,"v->name");
+ ncbytescat(origbuf,": ");
+ ncbytescat(origbuf,ncbytescontents(tmpbuf));
+ ncbytescat(origbuf,"\n");
+ /* capture */
+ buforig = ncbytesextract(origbuf);
+ /* Print any attributes */
+ if(v->natts > 0) {
+ ncbytesclear(tmpbuf);
+ testatts(ncid,v,tmpbuf);
+ attbuforig = ncbytesextract(tmpbuf);
+ }
+
+ /* Test copying */
if((stat=nc_copy_data_all(ncid,v->tid,v->data,v->dimprod,©))) return NCTHROW(stat);
- // Print copy
- if((stat=dumpvar(ncid,v,copy,&bufcopy))) return NCTHROW(stat);
- printf("(c) %s: %s\n",v->name,bufcopy);
+ ncbytesclear(tmpbuf);
+ if((stat=dumpvar(ncid,v,copy,tmpbuf))) return NCTHROW(stat);
+ ncbytescat(copybuf,"(o) ");
+ ncbytescat(copybuf,"v->name");
+ ncbytescat(copybuf,": ");
+ ncbytescat(copybuf,ncbytescontents(tmpbuf));
+ ncbytescat(copybuf,"\n");
+ /* capture */
+ bufcopy = ncbytesextract(copybuf);
+ /* Print any attributes */
+ if(v->natts > 0) {
+ ncbytesclear(tmpbuf);
+ testatts(ncid,v,tmpbuf);
+ attbufcopy = ncbytesextract(tmpbuf);
+ }
+
/* Compare */
if(strcmp(buforig,bufcopy) != 0)
fprintf(stderr,"*** orig != copy\n");
- if(buforig) free(buforig);
- if(bufcopy) free(bufcopy);
- // reclaim original
+ if(v->natts) {
+ if(strcmp(attbuforig,attbufcopy) != 0)
+ fprintf(stderr,"*** attribute orig != attribute copy\n");
+ }
+
+ /* reclaim original */
if((stat=nc_reclaim_data_all(ncid,v->tid,v->data,v->dimprod))) return NCTHROW(stat);
- // reclaim copy
+ /* reclaim copy */
if((stat=nc_reclaim_data_all(ncid,v->tid,copy,v->dimprod))) return NCTHROW(stat);
v->data = NULL;
+
+ ncbytesfree(tmpbuf);
+ ncbytesfree(origbuf);
+ ncbytesfree(copybuf);
+ if(buforig) free(buforig);
+ if(bufcopy) free(bufcopy);
+ if(attbuforig) free(attbuforig);
+ if(attbufcopy) free(attbufcopy);
+
return NCTHROW(stat);
}
@@ -307,6 +432,8 @@ test(struct Metadata* md)
CHECK(testvar(md->ncid,&md->vcstrvar));
CHECK(testvar(md->ncid,&md->vcvlenvar));
CHECK(testvar(md->ncid,&md->vccmpdvar));
+ /* Specials */
+ CHECK(testvar(md->ncid,&md->int3110));
done:
return;
}