GuiMeshRoadEditorCtrl.cpp little but dangerous mistake
by Andrea Barolo · in Torque 3D Professional · 04/17/2010 (1:38 am) · 5 replies
Hello all,
I noticed a little code mistake on GuiMeshRoadEditorCtrl.cpp , exactly on line 210:
the piece of code is as follow:
Assigning -1 on a unsigned int could result in a strange behavior.
For example my debugger transformed the insertNodeIdx into 213213 !!!
I hope it will be useful.
Have a nice week end !
I noticed a little code mistake on GuiMeshRoadEditorCtrl.cpp , exactly on line 210:
the piece of code is as follow:
MeshRoad *pRoad = NULL; MeshRoad *pClickedRoad = NULL; U32 insertNodeIdx = -1; //<------- THE DANGER !!! Point3F collisionPnt;
Assigning -1 on a unsigned int could result in a strange behavior.
For example my debugger transformed the insertNodeIdx into 213213 !!!
I hope it will be useful.
Have a nice week end !
About the author
#2
Precisely on tVector.h on line 117 !!
I say this because for some reason (using a strange shape created by me) the code on line 726 of tsShape.cpp
Giving -1 to the code of tVector.h gives to me U32 (i) = 42254222
Causing a out of bounds array access! error !!
I don't know if I expressed well !!
04/28/2010 (10:12 am)
Ok !! But I discovered another much more dangerous !!!Precisely on tVector.h on line 117 !!
T& operator[](S32 i) { return operator[](U32(i)); } //another S32 to U32 DANGER !!
const T& operator[](S32 i ) const { return operator[](U32(i)); }I say this because for some reason (using a strange shape created by me) the code on line 726 of tsShape.cpp
if (object->nodeIndex<0)
mesh->computeBounds(idMat,box);
else
mesh->computeBounds(gTempNodeTransforms[object->nodeIndex-start],box); // nodeindex =0 and start = 1 so 0-1= -1Giving -1 to the code of tVector.h gives to me U32 (i) = 42254222
Causing a out of bounds array access! error !!
I don't know if I expressed well !!
#3
you are half right, the casting of S32 to U32 in tVector.h helps to reduce code, because the access operator has only to be written once!
This results in an old programmer fashion, where all accessed data via signed integers. But a field cannot have the position -1 or -2 etc. it starts at 0 and ends with 2^32 (given enough memory ;-) )
So a better way to access a field etc is using the manner:
For your example define nodeIndex as U32 and use following code:
Hope I understand all right and it helps you solving your problem.
Kind regards Tobias!
PS: Just keep in mind:
Don't touch any vectors with S32
and
Check boundary with size()... ;-)
04/28/2010 (10:42 pm)
Good morning Andrea,you are half right, the casting of S32 to U32 in tVector.h helps to reduce code, because the access operator has only to be written once!
This results in an old programmer fashion, where all accessed data via signed integers. But a field cannot have the position -1 or -2 etc. it starts at 0 and ends with 2^32 (given enough memory ;-) )
So a better way to access a field etc is using the manner:
Vector<String> vec;
if( nodeIndex < vec.length() )
return vec[nodeIndex];
else
return "Empty String";For your example define nodeIndex as U32 and use following code:
if( object->nodeIndex-start < gTempNodeTransforms.size() )
mesh->computeBounds(gTempNodeTransforms[object->nodeIndex-start],box);
else
mesh->computeBounds(idMat,box);If start > nodeIndex nothing will happen you won't get a fault.Hope I understand all right and it helps you solving your problem.
Kind regards Tobias!
PS: Just keep in mind:
Don't touch any vectors with S32
and
Check boundary with size()... ;-)
#4
04/30/2010 (1:08 am)
Understood , thank you !!!
#5
06/05/2010 (11:46 am)
Problem Resolved. 
Torque Owner Tobias B.
mercatronics
this isn´t a real fault and will not lead to a strange behaviour, stepping through the code you will see, that the Idx will only be used for value return of a function call and not used any more. The initialisation is not really necessary for the purpose the variable was used. (But nice the programmer did it :-) )
Nice seen, but not really a fault!