kokoye2007 / waitzar

Automatically exported from code.google.com/p/waitzar
Other
0 stars 1 forks source link

Key Magic pointer corruption #136

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
The KeyMagic data structures use a series of references and pointers to avoid 
copying memory. However, something's definitely causing a pointer error; 
calling unrelated functions will crash the program by corrupting a totally 
unrelated array.

We need to re-think our data structures anyway; introducing a small amount of 
extra copying when LOADING them is nothing compared to slowdown when ACCESSING 
them; they remain read-only after loading anyway. 

Since we know roughly what we want from this code anyway, I intend to re-write 
it with setters that take const references, and getters that return const 
pointers. I'll also double-check the code for pointer corruption, and try to 
remove any assignment references which require us to pass in empty vectors, 
etc. 

Why is there always a memory corruption error JUST before release? :P

Original issue reported on code.google.com by seth.h...@gmail.com on 31 Aug 2010 at 9:46

GoogleCodeExporter commented 9 years ago
The enums, along with Rule, RuleSet, and Group, all use simple copying 
semantics. No need to change these (phew!); also, the load/save functions deal 
entirely with these.

Matcher and, by extension, Candidate use the reference:
   std::vector<Rule>& rulestream;

...and:
   std::vector<Rule>& replacementRules

The intention is to make a reference to the rules that already exist. However, 
this is just plain stupid. I think I was afraid of pointers at that point, 
because of the 1.7 pointer bug. 

We can use something like this:
  const Rule* ptrToRule;
... to signify that the Rule should not be modified, but the pointer itself can 
(i.e., by setting it to NULL).

We can also use:
  const Rule* const constPtrToRule;
...if we ever want to imply that the pointer can also never be changed. 

In addition, by using pointers, we can have NULL for "no match". 

Since I originally wanted to use const references, I'll try to replace the bad 
code with double-const ref's. If that fails, I'll go just for constant data, 
and let the pointer itself change. Either one should fix this mess, with 
minimal code changes. 

Original comment by seth.h...@gmail.com on 31 Aug 2010 at 9:59

GoogleCodeExporter commented 9 years ago
Fixed. We were returning refs to Candidates that were in a temporary array. 
Wouldn't have mattered, except that we copy-constructed them into the array.
Could have made the array global; instead, just returned a copy of the 
Candidate and a flag to indicate if it was valid or not.

No more pointer corruption... however, now KM is a bit slow in WZ. (unrelated)

Original comment by seth.h...@gmail.com on 1 Sep 2010 at 10:08