From 508eb3d9970fcd5e889508fae092d72d366dd583 Mon Sep 17 00:00:00 2001 From: Nicklas Larsson Date: Fri, 12 Apr 2024 12:37:43 +0200 Subject: [PATCH 1/2] libvector/vedit: fix memory leaks --- lib/vector/vedit/break.c | 3 --- lib/vector/vedit/delete.c | 4 ++-- lib/vector/vedit/merge.c | 2 ++ lib/vector/vedit/render.c | 4 +++- lib/vector/vedit/snap.c | 4 ++-- lib/vector/vedit/vertex.c | 7 +++---- lib/vector/vedit/zbulk.c | 4 +++- 7 files changed, 15 insertions(+), 13 deletions(-) diff --git a/lib/vector/vedit/break.c b/lib/vector/vedit/break.c index dd8612fa675..63f63c3eb10 100644 --- a/lib/vector/vedit/break.c +++ b/lib/vector/vedit/break.c @@ -41,14 +41,12 @@ int Vedit_split_lines(struct Map_info *Map, struct ilist *List, struct line_pnts *Points, *Points2; struct line_cats *Cats; - struct ilist *List_in_box; nlines_modified = 0; Points = Vect_new_line_struct(); Points2 = Vect_new_line_struct(); Cats = Vect_new_cats_struct(); - List_in_box = Vect_new_list(); for (i = 0; i < List->n_values; i++) { line = List->value[i]; @@ -130,7 +128,6 @@ int Vedit_split_lines(struct Map_info *Map, struct ilist *List, Vect_destroy_line_struct(Points); Vect_destroy_line_struct(Points2); Vect_destroy_cats_struct(Cats); - Vect_destroy_list(List_in_box); return nlines_modified; } diff --git a/lib/vector/vedit/delete.c b/lib/vector/vedit/delete.c index 7b2ead3362c..74a7358d777 100644 --- a/lib/vector/vedit/delete.c +++ b/lib/vector/vedit/delete.c @@ -94,8 +94,6 @@ int Vedit_delete_area(struct Map_info *Map, int area) int i, line, centroid, left, right; struct ilist *list; - list = Vect_new_list(); - G_debug(3, "Vedit_delete_area(): area=%d", area); centroid = Vect_get_area_centroid(Map, area); if (centroid != 0) { @@ -105,6 +103,7 @@ int Vedit_delete_area(struct Map_info *Map, int area) G_warning(_("Area %d without centroid"), area); return 0; } + list = Vect_new_list(); Vect_get_area_boundaries(Map, area, list); if (list->n_values > 0) { for (i = 0; i < list->n_values; i++) { @@ -119,6 +118,7 @@ int Vedit_delete_area(struct Map_info *Map, int area) } else { G_warning(_("Area %d has no boundaries"), area); + Vect_destroy_list(list); return 0; } diff --git a/lib/vector/vedit/merge.c b/lib/vector/vedit/merge.c index 9200c990ba1..1140f15d30e 100644 --- a/lib/vector/vedit/merge.c +++ b/lib/vector/vedit/merge.c @@ -203,6 +203,8 @@ int Vedit_merge_lines(struct Map_info *Map, struct ilist *List) Vect_destroy_cats_struct(Cats1); Vect_destroy_cats_struct(Cats2); + Vect_destroy_list(List_in_box); + return nlines_merged; } diff --git a/lib/vector/vedit/render.c b/lib/vector/vedit/render.c index a1c5bd8458e..9bd5462d1d7 100644 --- a/lib/vector/vedit/render.c +++ b/lib/vector/vedit/render.c @@ -204,8 +204,10 @@ struct robject *draw_line(struct Map_info *Map, int line, int draw_flag) G_debug(3, " draw_line(): type=%d rtype=%d npoints=%d draw=%d", state.type, obj->type, state.Points->n_points, draw); - if (!draw) + if (!draw) { + G_free(obj); return NULL; + } obj->npoints = state.Points->n_points; obj->point = diff --git a/lib/vector/vedit/snap.c b/lib/vector/vedit/snap.c index c8f51ea86fb..05e338ea67e 100644 --- a/lib/vector/vedit/snap.c +++ b/lib/vector/vedit/snap.c @@ -103,13 +103,13 @@ int Vedit_snap_line(struct Map_info *Map, struct Map_info **BgMap, int nbgmaps, struct line_cats *Cats; - Cats = Vect_new_cats_struct(); - G_debug(3, "Vedit_snap_line(): thresh=%g, to_vertex=%d", thresh, to_vertex); if (line > 0 && !Vect_line_alive(Map, line)) return -1; + Cats = Vect_new_cats_struct(); + npoints = Points->n_points; x = Points->x; y = Points->y; diff --git a/lib/vector/vedit/vertex.c b/lib/vector/vedit/vertex.c index 851257cd874..b58114481fc 100644 --- a/lib/vector/vedit/vertex.c +++ b/lib/vector/vedit/vertex.c @@ -44,14 +44,13 @@ int Vedit_move_vertex(struct Map_info *Map, struct Map_info **BgMap, double *x, *y, *z; char *moved; - struct line_pnts *Points, *Points_snap; + struct line_pnts *Points; struct line_cats *Cats; nvertices_moved = nvertices_snapped = 0; moved = NULL; Points = Vect_new_line_struct(); - Points_snap = Vect_new_line_struct(); Cats = Vect_new_cats_struct(); for (i = 0; i < List->n_values; i++) { @@ -157,6 +156,7 @@ int Vedit_move_vertex(struct Map_info *Map, struct Map_info **BgMap, if (rewrite) { if (Vect_rewrite_line(Map, line, type, Points, Cats) < 0) { + G_free(moved); return -1; } } @@ -164,9 +164,8 @@ int Vedit_move_vertex(struct Map_info *Map, struct Map_info **BgMap, /* destroy structures */ Vect_destroy_line_struct(Points); - Vect_destroy_line_struct(Points_snap); Vect_destroy_cats_struct(Cats); - /* G_free ((void *) moved); */ + G_free(moved); return nvertices_moved; } diff --git a/lib/vector/vedit/zbulk.c b/lib/vector/vedit/zbulk.c index 0d24c6d4b7a..f78a0d36fae 100644 --- a/lib/vector/vedit/zbulk.c +++ b/lib/vector/vedit/zbulk.c @@ -52,7 +52,6 @@ int Vedit_bulk_labeling(struct Map_info *Map, struct ilist *List, double x1, value = start; - Points = Vect_new_line_struct(); Points_se = Vect_new_line_struct(); Cats = Vect_new_cats_struct(); @@ -69,6 +68,8 @@ int Vedit_bulk_labeling(struct Map_info *Map, struct ilist *List, double x1, return -1; } + Points = Vect_new_line_struct(); + Vect_line_box(Points_se, &box_se); /* determine order of lines */ @@ -126,6 +127,7 @@ int Vedit_bulk_labeling(struct Map_info *Map, struct ilist *List, double x1, } if (Vect_delete_line(Map, temp_line) < 0) { + Vect_destroy_line_struct(Points); return -1; } From f13a724e1fc50d18cdfa1d34f6a4d65b2295032e Mon Sep 17 00:00:00 2001 From: Nicklas Larsson Date: Mon, 17 Jun 2024 12:22:04 +0200 Subject: [PATCH 2/2] address review suggestions and some more --- lib/vector/vedit/break.c | 7 +++++-- lib/vector/vedit/merge.c | 4 +++- lib/vector/vedit/snap.c | 4 +++- lib/vector/vedit/vertex.c | 5 +++-- lib/vector/vedit/zbulk.c | 13 +++++++------ 5 files changed, 21 insertions(+), 12 deletions(-) diff --git a/lib/vector/vedit/break.c b/lib/vector/vedit/break.c index 63f63c3eb10..5387bd3f016 100644 --- a/lib/vector/vedit/break.c +++ b/lib/vector/vedit/break.c @@ -99,7 +99,8 @@ int Vedit_split_lines(struct Map_info *Map, struct ilist *List, else newline = Vect_write_line(Map, type, Points2, Cats); if (newline < 0) { - return -1; + nlines_modified = -1; + goto free_exit; } if (List_updated) Vect_list_append(List_updated, newline); @@ -116,7 +117,8 @@ int Vedit_split_lines(struct Map_info *Map, struct ilist *List, /* rewrite the line */ newline = Vect_write_line(Map, type, Points2, Cats); if (newline < 0) { - return -1; + nlines_modified = -1; + goto free_exit; } if (List_updated) Vect_list_append(List_updated, newline); @@ -125,6 +127,7 @@ int Vedit_split_lines(struct Map_info *Map, struct ilist *List, } /* for each bounding box */ } /* for each selected line */ +free_exit: Vect_destroy_line_struct(Points); Vect_destroy_line_struct(Points2); Vect_destroy_cats_struct(Cats); diff --git a/lib/vector/vedit/merge.c b/lib/vector/vedit/merge.c index 1140f15d30e..7c752867232 100644 --- a/lib/vector/vedit/merge.c +++ b/lib/vector/vedit/merge.c @@ -184,7 +184,8 @@ int Vedit_merge_lines(struct Map_info *Map, struct ilist *List) if (Points->n_points > 0) { line = Vect_rewrite_line(Map, line1, type1, Points, Cats1); if (line < 0) { - return -1; + nlines_merged = -1; + goto free_exit; } if (line1 <= nlines) @@ -195,6 +196,7 @@ int Vedit_merge_lines(struct Map_info *Map, struct ilist *List) } } /* for each line */ +free_exit: /* destroy structures */ Vect_destroy_line_struct(Points1); Vect_destroy_line_struct(Points2); diff --git a/lib/vector/vedit/snap.c b/lib/vector/vedit/snap.c index 05e338ea67e..f60a21069cf 100644 --- a/lib/vector/vedit/snap.c +++ b/lib/vector/vedit/snap.c @@ -190,13 +190,15 @@ int Vedit_snap_lines(struct Map_info *Map, struct Map_info **BgMap, int nbgmaps, if (Vedit_snap_line(Map, BgMap, nbgmaps, line, Points, thresh, to_vertex) == 1) { if (Vect_rewrite_line(Map, line, type, Points, Cats) < 0) { - return -1; + nlines_modified = -1; + goto free_exit; } nlines_modified++; } } +free_exit: Vect_destroy_line_struct(Points); Vect_destroy_cats_struct(Cats); diff --git a/lib/vector/vedit/vertex.c b/lib/vector/vedit/vertex.c index b58114481fc..f64a10bee8a 100644 --- a/lib/vector/vedit/vertex.c +++ b/lib/vector/vedit/vertex.c @@ -156,12 +156,13 @@ int Vedit_move_vertex(struct Map_info *Map, struct Map_info **BgMap, if (rewrite) { if (Vect_rewrite_line(Map, line, type, Points, Cats) < 0) { - G_free(moved); - return -1; + nvertices_moved = -1; + goto free_exit; } } } /* for each selected line */ +free_exit: /* destroy structures */ Vect_destroy_line_struct(Points); Vect_destroy_cats_struct(Cats); diff --git a/lib/vector/vedit/zbulk.c b/lib/vector/vedit/zbulk.c index f78a0d36fae..cfa0d4becdc 100644 --- a/lib/vector/vedit/zbulk.c +++ b/lib/vector/vedit/zbulk.c @@ -52,6 +52,7 @@ int Vedit_bulk_labeling(struct Map_info *Map, struct ilist *List, double x1, value = start; + Points = Vect_new_line_struct(); Points_se = Vect_new_line_struct(); Cats = Vect_new_cats_struct(); @@ -65,11 +66,10 @@ int Vedit_bulk_labeling(struct Map_info *Map, struct ilist *List, double x1, /* write temporary line */ temp_line = Vect_write_line(Map, GV_LINE, Points_se, Cats); if (temp_line < 0) { - return -1; + nlines_modified = -1; + goto free_exit; } - Points = Vect_new_line_struct(); - Vect_line_box(Points_se, &box_se); /* determine order of lines */ @@ -119,7 +119,8 @@ int Vedit_bulk_labeling(struct Map_info *Map, struct ilist *List, double x1, } if (Vect_rewrite_line(Map, line, type, Points, Cats) < 0) { - return -1; + nlines_modified = -1; + goto free_exit; } nlines_modified++; @@ -127,10 +128,10 @@ int Vedit_bulk_labeling(struct Map_info *Map, struct ilist *List, double x1, } if (Vect_delete_line(Map, temp_line) < 0) { - Vect_destroy_line_struct(Points); - return -1; + nlines_modified = -1; } +free_exit: db_CatValArray_free(&cv); Vect_destroy_line_struct(Points); Vect_destroy_line_struct(Points_se);