Fix uninitialized, scope, leak, const issues in C (#2250)

* make_loc.c: reduce the scope of the variable 'path'
* make_loc.c: remove useless assignment of l_1 and l_2 since they are overwritten after
* make_loc.c: l_1 and l_2 are already checked before
* xmode.c: mode: initialize mode_v to avoid a false-positive warning
* xnmode.c: mode: initialize mode_v to avoid a false-positive warning
* gs3.c: Gs_update_attrange: initialize min and max to avoid a false-positive warning
* gs3.c: Gs_load_3dview: reduce scope of the variable 'pt'
* gs3.c: Gs_load_3dview: const geosurf
* mm_utils: MEMORY_LOG: pass str by reference
* dataquad.c: quad_data_new: fix memory leak data
* dataquad.c: quad_compare: avoid use of null data
* dataquad.c: quad_add_data: reduce scope of variables
* cr_from_a.c: main: fix resource leak: fp
* i.atcorr: various const ref performance
* bitmap.c: fix memory leak CWE: 401
* bitmap/sparse.c: fix memory leak CWE: 401
* sparse.c: BM_set_sparse: Tcount is never used
* sparse.c: BM_file_write_sparse: reduce scope of cnt
* r.terraflow/sweep.h: performs initialization in initialization list
* add missing const, various const ref performance

Issues detected by clang and cppcheck.

imagery/i.atcorr/main.cpp

@@ -237,7 +237,7 @@ class TICache
 
     }
 
-    void add(TransformInput ti, double alt, double vis)
+    void add(const TransformInput &ti, double alt, double vis)
     {
 	struct RBitem insert_ti = set_alt_vis(alt, vis);
 

imagery/i.atcorr/output.h

@@ -19,7 +19,7 @@ class Output
 	}
 
 	/* print a string */
-	static void Print(std::string x)			
+	static void Print(const std::string &x)
 	{ 
 		pos += x.length();
         fprintf(stderr, "%s", x.c_str());
@@ -51,7 +51,7 @@ class Output
 	}
 
 	/* write a s after cnt spaces */
-	static void WriteLn(int cnt, std::string s)
+	static void WriteLn(int cnt, const std::string &s)
 	{
 		Begin();
 		Repeat(cnt,' ');

imagery/i.atcorr/transform.cpp

@@ -132,7 +132,7 @@ void EtmDN(int iwave, double asol, bool before, double &lmin, double &lmax)
 /* Assuming input value between 0 and 1
    if rad is true, idn should first be converted to a reflectance value
    returns adjusted value also between 0 and 1 */
-double transform(const TransformInput ti, InputMask imask, double idn)
+double transform(const TransformInput &ti, InputMask imask, double idn)
 {
     /* convert from radiance to reflectance */
     if((imask & ETM_BEFORE) || (imask & ETM_AFTER))

imagery/i.atcorr/transform.h

@@ -42,6 +42,6 @@ enum InputMask
 /* Assuming input value between 0 and 1
 if rad is true, idn should first be converted to a reflectance value
 returns adjusted value also between 0 and 1 */
-extern double transform(const TransformInput ti, InputMask imask, double idn);
+extern double transform(const TransformInput &ti, InputMask imask, double idn);
 
 #endif /* TRANSFORM_H */

include/grass/defs/ogsf.h

@@ -310,7 +310,7 @@ int Gs_get_cat_label(const char *, int, int, char *);
 int Gs_save_3dview(const char *, geoview *, geodisplay *, struct Cell_head *,
 		   geosurf *);
 int Gs_load_3dview(const char *, geoview *, geodisplay *, struct Cell_head *,
-		   geosurf *);
+		   const geosurf *);
 int Gs_update_attrange(geosurf *, int);
 
 /* Gv3.c */

include/grass/iostream/mm_utils.h

@@ -45,6 +45,6 @@ void  LOG_avail_memo();
 
 size_t getAvailableMemory();
 
-void  MEMORY_LOG(std::string str);
+void  MEMORY_LOG(const std::string &str);
 
 #endif

lib/bitmap/bitmap.c

@@ -69,9 +69,12 @@ struct BM *BM_create(int x, int y)
 
     map->bytes = (x + 7) / 8;
 
-    if (NULL ==
-	(map->data = (unsigned char *)calloc(map->bytes * y, sizeof(char))))
+    void *tmp_map_data = (unsigned char *)calloc(map->bytes * y, sizeof(char));
+    if (tmp_map_data == NULL){
+        free(map);
 	return (NULL);
+    }
+    map->data = tmp_map_data;
 
     map->rows = y;
     map->cols = x;
@@ -324,7 +327,10 @@ struct BM *BM_file_read(FILE * fp)
 
     fread(&c, sizeof(char), sizeof(char), fp);
     if (c != BM_MAGIC)
+    {
+        free(map);
 	return NULL;
+    }
 
     fread(buf, BM_TEXT_LEN, sizeof(char), fp);
 
@@ -341,8 +347,11 @@ struct BM *BM_file_read(FILE * fp)
     if (map->sparse == BM_SPARSE)
 	goto readsparse;
 
-    if (NULL == (map->data = (unsigned char *)malloc(map->bytes * map->rows)))
+    if (NULL == (map->data = (unsigned char *)malloc(map->bytes * map->rows))) 
+    {
+        free(map);
 	return (NULL);
+    }
 
 
     for (i = 0; i < map->rows; i++)

lib/bitmap/sparse.c

@@ -51,8 +51,11 @@ struct BM *BM_create_sparse(int x, int y)
     map->bytes = (x + 7) / 8;
 
     if (NULL == (map->data = (unsigned char *)
-		 malloc(sizeof(struct BMlink *) * y)))
+		 malloc(sizeof(struct BMlink *) * y))) 
+    {
+        free(map);
 	return (NULL);
+    }
 
     map->rows = y;
     map->cols = x;
@@ -129,7 +132,7 @@ int BM_set_sparse(struct BM *map, int x, int y, int val)
 {
     struct BMlink *p, *p2, *prev;
     int cur_x = 0;
-    int Tcount, Tval;
+    int Tval;
     int dist_a, dist_b;
 
     val = !(!val);		/* set val == 1 or 0 */
@@ -141,7 +144,6 @@ int BM_set_sparse(struct BM *map, int x, int y, int val)
 	    if (p->val == val)	/* no change */
 		return 0;
 
-	    Tcount = p->count;	/* save current state */
 	    Tval = p->val;
 
 	    /* if x is on edge, then we probably want to merge it with 
@@ -366,7 +368,6 @@ int BM_file_write_sparse(FILE * fp, struct BM *map)
     char c;
     int i, y;
     struct BMlink *p;
-    int cnt;
 
     c = BM_MAGIC;
     fwrite(&c, sizeof(char), sizeof(char), fp);
@@ -383,7 +384,7 @@ int BM_file_write_sparse(FILE * fp, struct BM *map)
     for (y = 0; y < map->rows; y++) {
 	/* first count number of links */
 	p = ((struct BMlink **)(map->data))[y];
-	cnt = 0;
+	int cnt = 0;
 	while (p != NULL) {
 	    cnt++;
 	    p = p->next;

lib/calc/xmode.c

@@ -24,7 +24,12 @@ static int dcmp(const void *aa, const void *bb)
 
 static double mode(double *value, int argc)
 {
-    double mode_v;
+    /* Nota:
+     * It might be safer for to return nan or inf in case the input is empty,
+     * but it is a misuse of the function, so the return value is sort of
+     * undefined in that case.
+     */
+    double mode_v = 0.0;
     int mode_n = 0;
     int i;
 

lib/calc/xnmode.c

@@ -24,7 +24,12 @@ static int dcmp(const void *aa, const void *bb)
 
 static double mode(double *value, int argc)
 {
-    double mode_v;
+    /* Nota:
+     * It might be safer for to return nan or inf in case the input is empty,
+     * but it is a misuse of the function, so the return value is sort of
+     * undefined in that case.
+     */
+    double mode_v = 0.0;
     int mode_n = 0;
     int i;
 

lib/gis/make_loc.c

@@ -131,7 +131,6 @@ int G_make_location_epsg(const char *location_name,
 			 const struct Key_Value *proj_epsg)
 {
     int ret;
-    char path[GPATH_MAX];
 
     ret = G_make_location(location_name, wind, proj_info, proj_units);
 
@@ -140,6 +139,7 @@ int G_make_location_epsg(const char *location_name,
 
     /* Write out the PROJ_EPSG if available. */
     if (proj_epsg != NULL) {
+        char path[GPATH_MAX];
 	G_file_name(path, "", "PROJ_EPSG", "PERMANENT");
 	G_write_key_value_file(path, proj_epsg);
     }
@@ -436,7 +436,6 @@ int G_compare_projections(const struct Key_Value *proj_info1,
     /*      Do they have the same center latitude?                          */
     /* -------------------------------------------------------------------- */
 
-	l_1 = l_2 = NULL;
 	l_1 = G_find_key_value("lat_0", proj_info1);
 	l_2 = G_find_key_value("lat_0", proj_info2);
 
@@ -450,7 +449,6 @@ int G_compare_projections(const struct Key_Value *proj_info1,
     /*      Do they have the same standard parallels?                       */
     /* -------------------------------------------------------------------- */
 
-	l_1 = l_2 = NULL;
 	l_1 = G_find_key_value("lat_1", proj_info1);
 	l_2 = G_find_key_value("lat_1", proj_info2);
 
@@ -464,12 +462,11 @@ int G_compare_projections(const struct Key_Value *proj_info1,
 
 	    if (!l_2)
 		return -11;
-	    if (l_1 && l_2 && (fabs(atof(l_1) - atof(l_2)) > 0.000001)) {
+	    if (fabs(atof(l_1) - atof(l_2)) > 0.000001) {
 		return -11;
 	    }
 	}
 
-	l_1 = l_2 = NULL;
 	l_1 = G_find_key_value("lat_2", proj_info1);
 	l_2 = G_find_key_value("lat_2", proj_info2);
 
@@ -483,7 +480,7 @@ int G_compare_projections(const struct Key_Value *proj_info1,
 
 	    if (!l_2)
 		return -11;
-	    if (l_1 && l_2 && (fabs(atof(l_1) - atof(l_2)) > 0.000001)) {
+	    if (fabs(atof(l_1) - atof(l_2)) > 0.000001) {
 		return -11;
 	    }
 	}

lib/iostream/mm_utils.cpp

@@ -58,7 +58,7 @@ getAvailableMemory() {
 }
 
 void 
-MEMORY_LOG(std::string str) {
+MEMORY_LOG(const std::string &str) {
   printf("%s", str.c_str());
   fflush(stdout);
 }

lib/ogsf/gs3.c

@@ -949,12 +949,11 @@ int Gs_save_3dview(const char *vname, geoview * gv, geodisplay * gd,
    \return 1
  */
 int Gs_load_3dview(const char *vname, geoview * gv, geodisplay * gd,
-		   struct Cell_head *w, geosurf * defsurf)
+		   struct Cell_head *w, const geosurf * defsurf)
 {
     const char *mapset;
     struct G_3dview v;
     int ret = -1;
-    float pt[3];
 
     mapset = G_find_file2("3d.view", vname, "");
 
@@ -974,6 +973,7 @@ int Gs_load_3dview(const char *vname, geoview * gv, geodisplay * gd,
 
 	/* Set To and FROM positions */
 	/* TO */
+        float pt[3];
 	pt[0] = (v.from_to[TO][X] - w->west) - w->ew_res / 2.;
 	pt[1] = (v.from_to[TO][Y] - w->south) - w->ns_res / 2.;
 	pt[2] = v.from_to[TO][Z];
@@ -1087,7 +1087,8 @@ int Gs_load_3dview(const char *vname, geoview * gv, geodisplay * gd,
 int Gs_update_attrange(geosurf * gs, int desc)
 {
     size_t size;
-    float min, max;
+    float min = 0.0;
+    float max = 0.0;
     typbuff *tb;
     struct BM *nm;
     int found;

lib/rst/data/dataquad.c

@@ -79,8 +79,10 @@ struct quaddata *quad_data_new(double x_or, double y_or, double xmax,
     data->n_points = n_points;
     data->points =
 	(struct triple *)malloc(sizeof(struct triple) * (kmax + 1));
-    if (!data->points)
+    if (!data->points) {
+        free(data);
 	return NULL;
+    }
     for (i = 0; i <= kmax; i++) {
 	data->points[i].x = 0.;
 	data->points[i].y = 0.;
@@ -100,12 +102,12 @@ int quad_compare(struct triple *point, struct quaddata *data)
     int cond1, cond2, cond3, cond4, rows, cols;
     double ew_res, ns_res;
 
+    if (data == NULL)
+	return -1;
+
     ew_res = (data->xmax - data->x_orig) / data->n_cols;
     ns_res = (data->ymax - data->y_orig) / data->n_rows;
 
-
-    if (data == NULL)
-	return -1;
     if (data->n_rows % 2 == 0) {
 	rows = data->n_rows / 2;
     }
@@ -142,26 +144,24 @@ int quad_compare(struct triple *point, struct quaddata *data)
  */
 int quad_add_data(struct triple *point, struct quaddata *data, double dmin)
 {
-    int n, i, cond;
-    double xx, yy, r;
 
-    cond = 1;
+    int cond = 1;
     if (data == NULL) {
 	fprintf(stderr, "add_data: data is NULL \n");
 	return -5;
     }
-    for (i = 0; i < data->n_points; i++) {
-	xx = data->points[i].x - point->x;
-	yy = data->points[i].y - point->y;
-	r = xx * xx + yy * yy;
+    for (int i = 0; i < data->n_points; i++) {
+	double xx = data->points[i].x - point->x;
+	double yy = data->points[i].y - point->y;
+	double r = xx * xx + yy * yy;
 	if (r <= dmin) {
 	    cond = 0;
 	    break;
 	}
     }
 
     if (cond) {
-	n = (data->n_points)++;
+	int n = (data->n_points)++;
 	data->points[n].x = point->x;
 	data->points[n].y = point->y;
 	data->points[n].z = point->z;

lib/vector/dglib/examples/cr_from_a.c

@@ -81,6 +81,7 @@ int main(int argc, char **argv)
   reread_first_line:
     if (fgets(sz, sizeof(sz), fp) == NULL) {
 	fprintf(stderr, "unexpected EOF\n");
+        fclose(fp);
 	return 1;
     }
 

raster/r.terraflow/fill.cpp

@@ -138,7 +138,7 @@ class printDepth {
 
 
 char *
-verbosedir(std::string s) {
+verbosedir(const std::string &s) {
   static char buf[BUFSIZ];
   sprintf(buf, "dump/%s", s.c_str());
   return buf;
@@ -537,7 +537,7 @@ assignFinalDirections(AMI_STREAM<plateauStats> *statstr,
 class directionElevationMerger {
 public:
   waterGridType operator()(elevation_type el, direction_type dir, 
-			   waterType p) { 
+			   const waterType &p) { 
     /* check that no (boundary) nodata values got in here */
     assert(el != nodataType::ELEVATION_BOUNDARY);
     assert(!is_nodata(el));		/* p should be a valid grid cell */

raster/r.terraflow/sweep.h

@@ -498,10 +498,10 @@ class flowStructure  {
   /* flowStructure(const flowValue &e, const flowPriority &p):
 	 prio(p), val(e) {}
   */ 
-  flowStructure(const flowStructure &fl) {
-    prio = fl.prio;
-    val = fl.val;
-  }
+  flowStructure(const flowStructure &fl):
+    prio(fl.prio),
+    val(fl.val)
+  {}
 
   ~flowStructure() {}
 

raster/r.terraflow/water.h

@@ -319,7 +319,7 @@ class waterGridType : public waterWindowBaseType {
 		bfs_depth_type gdepth=DEPTH_INITIAL) :
     waterWindowBaseType(gel, gdir, gdepth), label(glabel) {
   }
-  waterGridType(elevation_type gel, waterType w) :
+  waterGridType(elevation_type gel, const waterType &w) :
     waterWindowBaseType(gel, w.dir, w.depth), label(w.label) {}
 
   cclabel_type getLabel() const { return label; };