Fixed dap memory leaks

libdispatch/nc_url.c

@@ -17,11 +17,13 @@ static void nc_urlparamfree(NClist* params);
 int
 nc_urlparse(const char* url0, NC_URL** ncurlp)
 {
-    char* url;
+    NCerror ncstat = NC_NOERR;
+    char* url = NULL;
     char* p;
     char* p1;
     int c;
     NC_URL* ncurl;
+    size_t protolen;
 
     /* accumulate parse points*/
     char* protocol = NULL;
@@ -47,7 +49,10 @@ nc_urlparse(const char* url0, NC_URL** ncurlp)
 	params = p+1;
 	/* find end of the clientparams*/
         for(;*p;p++) {if(p[0] == RBRACKET && p[1] != LBRACKET) break;}
-	if(*p == 0) return NC_EINVAL; /* malformed client params*/
+	if(*p == 0) {
+	    ncstat = NC_EINVAL; /* malformed client params*/
+	    goto done;
+	}
 	*p = '\0'; /* leave off the trailing rbracket for now */
 	p++; /* move past the params*/
     }
@@ -57,11 +62,16 @@ nc_urlparse(const char* url0, NC_URL** ncurlp)
     /* Note that we dont care what the protocol is ; just collect it */
     /* find the end of the protocol */
     p1 = strchr(p,':');
-    if(p1 == NULL || p1 == p) return NC_EINVAL; /* missing protocol*/
+    if(p1 == NULL || p1 == p) {
+	ncstat = NC_EINVAL; /* missing protocol*/
+        goto done;
+    }
     /* Simulate strndup */
-    protocol = malloc((size_t)(p1-p));
+    protolen = (size_t)(p1-p);
+    protocol = malloc(1+protolen);
     if(protocol == NULL) return NC_ENOMEM;
-    strncpy(protocol,p,(p1-p));
+    strncpy(protocol,p,protolen);
+    protocol[protolen] = '\0';
     /* Look for '?' */
     constraint = strchr(p,'?');
     if(constraint) {
@@ -95,8 +105,9 @@ nc_urlparse(const char* url0, NC_URL** ncurlp)
 		ncurl->params, ncurl->base, ncurl->projection, ncurl->selection);
 #endif
 
-    free(url);
-    return NC_NOERR;
+done:
+    if(url != NULL) free(url);
+    return ncstat;
 
 }
 

libncdap3/ncdap3a.c

@@ -43,6 +43,9 @@ cleanNCDAPCOMMON(NCDAPCOMMON* nccomm)
     /* free the trees */
     freecdfroot34(nccomm->cdf.ddsroot);
     nccomm->cdf.ddsroot = NULL;
+    if(nccomm->oc.ocdasroot != NULL)
+        oc_root_free(nccomm->oc.conn,nccomm->oc.ocdasroot);
+    nccomm->oc.ocdasroot = NULL;
     oc_close(nccomm->oc.conn); /* also reclaims remaining OC trees */
     dapurlclear(&nccomm->oc.url);
     efree(nccomm->oc.urltext);

nc_test/Makefile.am

@@ -41,6 +41,11 @@ TESTS = $(TESTPROGRAMS)
 # This will run a bunch of the test programs with valgrind, the memory
 # checking tool. (Valgrind must be present for this to work.)
 if USE_VALGRIND_TESTS
+if USE_NETCDF4
+TESTS_ENVIRONMENT=USE_NETCDF4=1
+else
+TESTS_ENVIRONMENT=USE_NETCDF4=0
+endif
 TESTS += run_valgrind_tests.sh
 endif # USE_VALGRIND_TESTS
 

nc_test/run_valgrind_tests.sh

@@ -9,7 +9,11 @@ echo ""
 echo "Testing programs with valgrind..."
 
 # These are my test programs.
-list='t_nc tst_atts tst_norm tst_names tst_misc nc_test'
+list='t_nc tst_norm tst_names tst_misc nc_test'
+# If we are running with netcdf4, then add tst_atts
+if test "x$USE_NETCDF4" = "x1" ; then
+list="$list tst_atts"
+fi
 
 # These don't work yet: tst_fills tst_xplatform2 tst_interops6 tst_strings 
 

nc_test4/Makefile.am

@@ -63,6 +63,11 @@ endif # USE_HDF4
 # This will run a bunch of the test programs with valgrind, the memory
 # checking tool. (Valgrind must be present for this to work.)
 if USE_VALGRIND_TESTS
+if USE_NETCDF4
+TESTS_ENVIRONMENT=USE_NETCDF4=1
+else
+TESTS_ENVIRONMENT=USE_NETCDF4=0
+endif
 TESTS += run_valgrind_tests.sh
 endif # USE_VALGRIND_TESTS
 

nc_test4/run_valgrind_tests.sh

@@ -9,8 +9,12 @@ echo ""
 echo "Testing programs with valgrind..."
 
 # These are my test programs.
-list='t_type tst_camrun tst_vl tst_v2 tst_vars2 tst_atts '\
-'tst_atts2 tst_files2 tst_files'
+list="t_type tst_camrun tst_vl tst_v2 tst_vars2 \
+tst_atts2 tst_files2 tst_files"
+# If we are running with netcdf4, then add tst_atts
+if test "x$USE_NETCDF4" = "x1" ; then
+list="$list tst_atts"
+fi
 
 for tst in $list; do
     echo ""

ncdap_test/Makefile.am

@@ -58,6 +58,8 @@ test_cvt3_SOURCES = test_cvt.c
 test_varm3_SOURCES = test_varm3.c
 check_PROGRAMS += test_cvt3 test_varm3
 TESTS += test_cvt3 test_varm3
+# Failing for some reason
+XFAIL_TESTS = test_varm3
 endif
 
 endif ENABLE_DAP_REMOTE_TESTS

ncdap_test/test_varm3.c

@@ -64,7 +64,7 @@ check(int status, char* file, int line)
 {
     if(status == 0) return;
     fprintf(stderr,"error: %s at %s:%d\n",nc_strerror(status),file,line);
-    exit(0); /* treat like xfail */
+    exit(1);
 }
 
 int
@@ -234,11 +234,11 @@ main()
     }
     printf("*** %s: stride case 3\n",(fail?"Fail":"Pass"));
 
-    return 0;
+    return fail;
 
 ncfail:
     printf("*** nc function failure: %d %s\n",err,nc_strerror(err));
-    return 0; /* treat like xfail */
+    return 1;
 }
 
 

oc/ocinternal.c

@@ -156,7 +156,8 @@ ocinternalinitialize(void)
         char* homepath = NULL;
 	FILE* f = NULL;
         /* locate the configuration files: . first, then $HOME */
-        path = (char*)malloc(strlen(".")+1+strlen(DODSRC)+1);
+        path = (char*)malloc(strlen("./")+strlen(DODSRC)+1);
+	if(path == NULL) return OC_ENOMEM;
         strcpy(path,"./");
         strcat(path,DODSRC);
 	/* see if file is readable */
@@ -165,7 +166,9 @@ ocinternalinitialize(void)
 	    /* try $HOME */
             homepath = getenv("HOME");
             if (homepath!= NULL) {
+	       if(path != NULL) free(path);
 	       path = (char*)malloc(strlen(homepath)+1+strlen(DODSRC)+1);
+	       if(path == NULL) return OC_ENOMEM;
 	       strcpy(path,homepath);
 	       strcat(path,"/");
 	       strcat(path,DODSRC);
@@ -242,6 +245,7 @@ ocfetch(OCstate* state, const char* constraint, OCdxd kind, OCnode** rootp)
     MEMCHECK(tree,OC_ENOMEM);
     memset((void*)tree,0,sizeof(OCtree));
     tree->dxdclass = kind;
+    tree->state = state;
     tree->constraint = constraintescape(constraint);
     if(tree->constraint == NULL)
 	tree->constraint = nulldup(constraint);
@@ -360,6 +364,9 @@ ocfetch(OCstate* state, const char* constraint, OCdxd kind, OCnode** rootp)
     }
 #endif
 
+    /* Put root into the state->trees list */
+    oclistpush(state->trees,(ocelem)root);
+
     if(rootp) *rootp = root;
     return stat;
 
@@ -374,6 +381,8 @@ occlose(OCstate* state)
     unsigned int i;
     if(state == NULL) return;
 
+    /* Warning: ocfreeroot will attempt to remove the root from state->trees */
+    /* Ok in this case because we are popping the root out of state->trees */
     for(i=0;i<oclistlength(state->trees);i++) {
 	OCnode* root = (OCnode*)oclistpop(state->trees);
 	ocfreeroot(root);

oc/ocnode.c

@@ -326,8 +326,20 @@ void
 ocfreeroot(OCnode* root)
 {
     OCtree* tree;
+    OCstate* state;
+    int i;
+
     if(root == NULL || root->tree == NULL) return;
+
     tree = root->tree;
+    /* Remove the root from the state->trees list */
+    state = tree->state;
+    for(i=0;i<oclistlength(state->trees);i++) {
+	OCnode* node = (OCnode*)oclistget(state->trees,i);
+	if(root == node)
+	    oclistremove(state->trees,i);
+    }
+    /* Note: it is ok if state->trees does not contain this root */    
     ocfreetree(tree);
 }