Looking at _pacc_coords(), I noticed that it seemed to have the same realloc() bug that I'd just fixed in _pacc_result(). However, the "list of arrays" trick really wasn't going to work here.
The problem is that we do a binary search on the collection of coordinates we've amassed, which means it must be a single array. Also, since the list is sorted we sometimes need to insert a new element in the middle. That would be incredibly hard to do with a list of arrays.
So after a bit of fretting I hit on a different approach: use a struct return to pass both values back from the function without involving pointers, then provide a couple of macros to extract the ints from the struct.
Stupidly, I implemented this without first writing a test case. Having got used to the safety harness of test-driven development, I fretted about this: was the bug I thought I'd seen really there? And if so, had I really fixed it? I had to go back and write one. The bug was there, although it proved surprisingly difficult to exercise. It depends on evaluating a coordinate, then holding onto that value as another coordinate (which causes the realloc()) is calculated.
My first attempts gave some odd results, and I realised there was another bug in _pacc_coords(). Due to complete silliness on my part, it never calculated the right column value for the second coordinate evaluated on a line. So that became a test case for a bug I previously hadn't noticed.
Next time round I finally managed to get a test case to exercise the realloc() bug. As with exercising memmove(), it requires contrived use of semantic guards to cause things to be evaluated in an unusual order.
At first, I'd convinced myself that I had a correct test case, but nothing failed (obviously I'd reverted to the version of the code that I thought had the bug). However, with careful use of first gdb and then valgrind I was able to see that there was a use of freed memory (which happened to still have the right values in it). How to handle this? I thought about setting up a new class of test case that grepped the output of valgrind for text like Invalid read.... But I hadn't gone far dawn that road when...
I remembered MALLOC_PERTURB_! This little-advertised feature of glibc was introduced to me by Jeff Johnson. Details in mallopt(3). Having once thought of it, I immediately realised that it should be enabled for all tests, which is easy to do. The good news is that this didn't reveal any new bugs in the parser engine, although it did show up a use-after-free bug that I'd introduced into the cooking code. For now, this has reverted to a memory leak. It really doesn't matter too much if we leak memory in the compiler, and I need to get a new release out.
Which I have now done. Whew!
Last updated: 2015-07-31 20:50:44 UTC
One thing pacc needs is more users. And, perhaps, one way to get more users is to reduce the friction in getting started with pacc. An obvious lubricant is packaging. Read More...
Looking at _pacc_coords(), I noticed that it seemed to have the same realloc() bug that I'd just fixed in _pacc_result(). However, the "list of arrays" trick really wasn't going to work here. Read More...