Hi all! I am currently trying to resolve a crash which can be reproduced with the attached input file "ReplaceDuplicatePoints_Bug.geo" when one runs:
gmsh ReplaceDuplicatePoints_Bug.geo -1 The crash occurs in "ReplaceDuplicatePoints" and I have narrowed down its cause to the insertion of the vertex 106500 into the Tree structure allNonDuplicatedPoints. A test which can demonstrate the problem is the following: 1. temporary store the vertex 19223 into a variable test_vertex 2. use the Tree_Search function to check for test_vertex in "allNonDuplicatedPoints" just before of the insertion of of the vertex 106500 into "allNonDuplicatedPoints" --> Reports that there is already an equivalent vertex stored. 3. repeat the test 2 just after the insertion of of the vertex 106500 into "allNonDuplicatedPoints" --> Reports that there is no equivalent vertex stored. It seems that the rebalancing of the tree changes some of its branches, which shouldn't be altered. Since the "ReplaceDuplicatePoints" function is a very fundamental part of gmsh, I believe it is very important to be very robust. I think that the way this function is implemented is not roundoff-error safe. Anyway I would appreciate any hint which could help me resolve this issue. By the way I would like to suggest 2 patches for the same function but not relevant with this specific problem. The first one is performance oriented and it removes some double calls of the Tree_search function. The second is about an eventual typo in freeing memory. Both patches are attached. Please review them. Best Regards Kostas
ReplaceDuplicatePoints_Bug.geo
Description: Binary data
--- ./gmsh-rev7517/Geo/Geo.cpp 2010-04-23 10:31:12.827489995 +0200
+++ ./gmsh-rev7517/Geo/Geo-1.cpp 2010-04-27 18:30:29.314840811 +0200
@@ -2739,10 +2739,7 @@ static void ReplaceDuplicatePoints()
allNonDuplicatedPoints = Tree_Create(sizeof(Vertex *), compareTwoPoints);
for(i = 0; i < List_Nbr(All); i++) {
List_Read(All, i, &v);
- if(!Tree_Search(allNonDuplicatedPoints, &v)) {
- Tree_Insert(allNonDuplicatedPoints, &v);
- }
- else {
+ if(!Tree_Insert(allNonDuplicatedPoints, &v)) {
Tree_Suppress(GModel::current()->getGEOInternals()->Points, &v);
//List_Add(points2delete,&v);
}
@@ -2839,8 +2836,7 @@ static void ReplaceDuplicateCurves()
for(i = 0; i < List_Nbr(All); i++) {
List_Read(All, i, &c);
if(c->Num > 0) {
- if(!Tree_Search(allNonDuplicatedCurves, &c)) {
- Tree_Insert(allNonDuplicatedCurves, &c);
+ if(Tree_Insert(allNonDuplicatedCurves, &c)) {
if(!(c2 = FindCurve(-c->Num))) {
Msg::Error("Unknown curve %d", -c->Num);
List_Delete(All);
@@ -2913,10 +2909,7 @@ static void ReplaceDuplicateSurfaces()
for(i = 0; i < List_Nbr(All); i++) {
List_Read(All, i, &s);
if(s->Num > 0) {
- if(!Tree_Search(allNonDuplicatedSurfaces, &s)) {
- Tree_Insert(allNonDuplicatedSurfaces, &s);
- }
- else {
+ if(!Tree_Insert(allNonDuplicatedSurfaces, &s)) {
Tree_Suppress(GModel::current()->getGEOInternals()->Surfaces, &s);
}
}
--- ./gmsh-rev7517/Geo/Geo-1.cpp 2010-04-27 18:30:29.314840000 +0200
+++ ./gmsh-rev7517/Geo/Geo-2.cpp 2010-04-27 19:50:00.006819538 +0200
@@ -2810,7 +2810,7 @@ static void ReplaceDuplicatePoints()
}
List_Delete(All);
- for(int k = 0; k < List_Nbr(points2delete); k++) {
+ for(i = 0; i < List_Nbr(points2delete); i++) {
List_Read(points2delete, i, &v);
Free_Vertex(&v, 0);
}
_______________________________________________ gmsh mailing list [email protected] http://www.geuz.org/mailman/listinfo/gmsh
