Browse Source

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.
Loïc Bartoletti 2 years ago
parent
commit
2bb77ed6a4

+ 1 - 1
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);
 

+ 2 - 2
imagery/i.atcorr/output.h

@@ -19,7 +19,7 @@ public:
 	}
 
 	/* 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 @@ public:
 	}
 
 	/* 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,' ');

+ 1 - 1
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))

+ 1 - 1
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 */

+ 1 - 1
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 */

+ 1 - 1
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

+ 12 - 3
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++)

+ 6 - 5
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;

+ 6 - 1
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;
 

+ 6 - 1
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;
 

+ 3 - 6
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;
 	    }
 	}

+ 1 - 1
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);
 }

+ 4 - 3
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;

+ 12 - 12
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,18 +144,16 @@ 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;
@@ -161,7 +161,7 @@ int quad_add_data(struct triple *point, struct quaddata *data, double dmin)
     }
 
     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;

+ 1 - 0
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;
     }
 

+ 2 - 2
raster/r.terraflow/fill.cpp

@@ -138,7 +138,7 @@ public:
 
 
 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 */

+ 4 - 4
raster/r.terraflow/sweep.h

@@ -498,10 +498,10 @@ public:
   /* 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() {}
   

+ 1 - 1
raster/r.terraflow/water.h

@@ -319,7 +319,7 @@ public:
 		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; };

+ 3 - 3
raster/r.viewshed/distribute.cpp

@@ -82,7 +82,7 @@ extern "C"
 IOVisibilityGrid *distribute_and_sweep(char *inputfname,
 				       GridHeader * hd,
 				       Viewpoint * vp,
-				       ViewOptions viewOptions)
+				       const ViewOptions &viewOptions)
 {
 
     assert(inputfname && hd && vp);
@@ -180,7 +180,7 @@ unsigned long distribute_sector(AMI_STREAM < AEvent > *eventList,
 				AMI_STREAM < AEvent > *enterBndEvents,
 				double start_angle, double end_angle,
 				IOVisibilityGrid * visgrid, Viewpoint * vp,
-				GridHeader *hd, ViewOptions viewOptions)
+				GridHeader *hd, const ViewOptions &viewOptions)
 {
 
 
@@ -549,7 +549,7 @@ unsigned long solve_in_memory(AMI_STREAM < AEvent > *eventList,
 			      AMI_STREAM < AEvent > *enterBndEvents,
 			      double start_angle, double end_angle,
 			      IOVisibilityGrid * visgrid, GridHeader *hd,
-			      Viewpoint * vp, ViewOptions viewOptions)
+			      Viewpoint * vp, const ViewOptions &viewOptions)
 {
 
     assert(eventList && visgrid && vp);

+ 3 - 3
raster/r.viewshed/distribute.h

@@ -51,7 +51,7 @@
 IOVisibilityGrid *distribute_and_sweep(char *inputfname,
 				       GridHeader * hd,
 				       Viewpoint * vp,
-				       ViewOptions viewOptions);
+				       const ViewOptions &viewOptions);
 
 
 
@@ -73,7 +73,7 @@ unsigned long distribute_sector(AMI_STREAM < AEvent > *eventList,
 				AMI_STREAM < AEvent > *enterBndEvents,
 				double start_angle, double end_angle,
 				IOVisibilityGrid * visgrid, Viewpoint * vp,
-				GridHeader *hd, ViewOptions viewOptions);
+				GridHeader *hd, const ViewOptions &viewOptions);
 
 /* bndEvents is a stream of events that cross into the sector's
    (first) boundary; they must be distributed to the boundary streams
@@ -101,7 +101,7 @@ unsigned long solve_in_memory(AMI_STREAM < AEvent > *eventList,
 			      AMI_STREAM < AEvent > *enterBndEvents,
 			      double start_angle, double end_angle,
 			      IOVisibilityGrid * visgrid, GridHeader *hd,
-			      Viewpoint * vp, ViewOptions viewOptions);
+			      Viewpoint * vp, const ViewOptions &viewOptions);
 
 
 /*returns 1 if enter angle is within epsilon from boundary angle */

+ 3 - 3
raster/r.viewshed/rbbst.cpp

@@ -87,7 +87,7 @@ TreeNode *NIL;
 
 
 /*public:--------------------------------- */
-RBTree *create_tree(TreeValue tv)
+RBTree *create_tree(const TreeValue &tv)
 {
     init_nil_node();
     RBTree *rbt = (RBTree *) G_malloc(sizeof(RBTree));
@@ -127,7 +127,7 @@ void destroy_sub_tree(TreeNode * node)
     return;
 }
 
-void insert_into(RBTree * rbt, TreeValue value)
+void insert_into(RBTree * rbt, const TreeValue &value)
 {
     insert_into_tree(&(rbt->root), value);
     return;
@@ -199,7 +199,7 @@ char compare_double(double a, double b)
 
 
 /*create a tree node */
-TreeNode *create_tree_node(TreeValue value)
+TreeNode *create_tree_node(const TreeValue &value)
 {
     TreeNode *ret;
 

+ 3 - 3
raster/r.viewshed/rbbst.h

@@ -84,10 +84,10 @@ typedef struct rbtree_
 
 
 
-RBTree *create_tree(TreeValue tv);
+RBTree *create_tree(const TreeValue &tv);
 void delete_tree(RBTree * t);
 void destroy_sub_tree(TreeNode * node);
-void insert_into(RBTree * rbt, TreeValue value);
+void insert_into(RBTree * rbt, const TreeValue &value);
 void delete_from(RBTree * rbt, double key);
 TreeNode *search_for_node_with_key(RBTree * rbt, double key);
 
@@ -135,7 +135,7 @@ char compare_values(TreeValue * v1, TreeValue * v2);
 char compare_double(double a, double b);
 
 /*create a tree node */
-TreeNode *create_tree_node(TreeValue value);
+TreeNode *create_tree_node(const TreeValue &value);
 
 /*create node with its value set to the value given
    //and insert the node into the tree */

+ 2 - 2
raster/r.viewshed/visibility.cpp

@@ -89,7 +89,7 @@ void copy_viewpoint(Viewpoint * a, Viewpoint b)
 /* MemoryVisibilityGrid functions */
 
 /* create and return a grid of the sizes specified in the header */
-MemoryVisibilityGrid *create_inmem_visibilitygrid(GridHeader hd, Viewpoint vp)
+MemoryVisibilityGrid *create_inmem_visibilitygrid(const GridHeader &hd, Viewpoint vp)
 {
 
     MemoryVisibilityGrid *visgrid;
@@ -284,7 +284,7 @@ void save_inmem_visibilitygrid(MemoryVisibilityGrid * visgrid,
 
 /* ------------------------------------------------------------ */
 /*create grid from given header and viewpoint */
-IOVisibilityGrid *init_io_visibilitygrid(GridHeader hd, Viewpoint vp)
+IOVisibilityGrid *init_io_visibilitygrid(const GridHeader &hd, Viewpoint vp)
 {
     IOVisibilityGrid *visgrid;
 

+ 2 - 2
raster/r.viewshed/visibility.h

@@ -211,7 +211,7 @@ void set_viewpoint_elev(Viewpoint * vp, float elev);
 /* ------------------------------------------------------------ */
 /* MemoryVisibilityGrid functions */
 
-MemoryVisibilityGrid *create_inmem_visibilitygrid(GridHeader hd,
+MemoryVisibilityGrid *create_inmem_visibilitygrid(const GridHeader &hd,
 						  Viewpoint vp);
 
 void free_inmem_visibilitygrid(MemoryVisibilityGrid * visgrid);
@@ -230,7 +230,7 @@ void save_inmem_visibilitygrid(MemoryVisibilityGrid * vigrid,
 /* IOVisibilityGrid functions */
 
 /*create grid from given header and viewpoint */
-IOVisibilityGrid *init_io_visibilitygrid(GridHeader hd, Viewpoint vp);
+IOVisibilityGrid *init_io_visibilitygrid(const GridHeader &hd, Viewpoint vp);
 
 /*frees a visibility grid */
 void free_io_visibilitygrid(IOVisibilityGrid * grid);