Mail Archives: geda-user/2017/08/16/03:56:18
Hi all,
if you ever dared to dive the code of pcb or pcb-rnd, you probably
remember a strange construct: many of the function calls take the
following arguments:
int Type, void *Ptr1, void *Ptr2, void *Ptr3
or save data in structs or local variabls with the same types/names.
This mail, that later will be converted to a devlog, gives an explanation
on what it is, when it happened, why I think it is a bad idea and how it
is being fixed in pcb-rnd.
1. What it is
The data model of pcb is a tree, but it is very rigid and restricted. A
line is always on a layer, a pin is always in an element. A layer and an
element is always in a buffer (e.g. board). The structure of the tree is
not just a convention: it's hardwired in every piece of the code. E.g. if
you have a line, the code knows/assumes the parent must be a layer, it
can't be anything else.
Thus there is "no need for objects to remember their own type or their
place in the tree, e.g. remembering their parent".
Instead all searches are done from up to down, e.g. if we are looking for
pins, we start from buffer (board) level and we are first iterating over
elements and then pins within elements. By the time we find a pin, we have
3 info at hand, in local variables:
- the fact that the object we found is a PIN (this is going to be int
Type)
- the element the pin lives in (this is going to be void *Ptr1)
- the pin itself (this is going to be void *Ptr2)
If we are to pass the pin to the next function, we need to pass these
info, thus we pass Type, Ptr1, Ptr2 ... and Ptr3.
Ptr3 is unused most of the time; when it is not unused, it's often a
"property" or child (in the tree) of Ptr2; e.g. when addressing an
endpoint of a line, Ptr2 is the line and Ptr3 refers to the endpoint.
To maximize confusion, when Ptr3 is unused, it's value usually matches
Ptr2's and when only one pointer is needed (e.g. in case of an element)
all 3 pointers are the same.
2. Why is it really a bad idea
Reason #1: theoretical: one of the features C provides over e.g. asm is
types: the compiler can understand a bit more about why we do what we do.
We have a dedicated type to store the data of a pin, another for line and
a third for element. If we accidentally compare a line to a pin or we try
to say something like element = pad, because of the mismatching types, the
compiler can give a nice warning and we know we did something wrong,
without having to wait for it to bite us runtime.^1
A void * is a way to tell the compiler not to care. Since many of those
functions need to be able to handle different object types, the original
authors though it would be a good idea to tell the compierl not to care
about the object type at all. The code then looks at the "int Type" arg
and _assumes_ ptr1's and ptr2's type, knowing the rigid setup of the
tree...
Reason #2: ... which makes it very very easy to introduce bugs: both the
caller and the callee need to have the very same assumptions in every
single call/function pair. Pass the wrong object in any pointer and the
software starts to misbehave in unexpected ways. There are over 40
such type/ptr1/ptr2/ptr3 functions in pcb 4.0.0. The assumptions are not
very well documented either.
Reason #3: debugging; modern debuggers like gdb are very handy: they
understand a lot of the C syntax and get enough debug info embedded in the
executable that they can explore structures and understand data types.
Unless we tell them to not do any of that, by using void *.
Reason #4: the combination of the above 3: when it breaks, it's a
nightmare to figure how it broke. Some reasonable looking part of the code
starts doing something with a pin that looks broken. But tracing back
where the pin broke is often a challenge, as we never know if a given
level of function calls (stack frame) ment it to be a pin or an arc, or
thought that ptr2 is just unused while the callee expected it to be
used...
Reason #5: since these assumptions about the tree are hardwired
everywhere, it's rather hard to make upgrades to the tree (like the
groupping concept subcircuits introduced in pcb-rnd, or just adding yet
another object type). It's especially too easy to miss a new
possible combination of ptr1/ptr2 in one of those many calls. For a long
time this was one of the main reasons I didn't start the data model
cleanup in pcb-rnd.
3. History: when did this happen?
The earliest pcb version I found in pcb-history^2 that already showed
consistent use of this idiom is version 1.3. That means 1995. So this
means "we had this all the time".
4. How it is being solved in pcb-rnd
Both in pcb and pcb-rnd there's a common header every object struct has.
It contains things like the uniqueu ID of the object.
This common header is extended in pcb-rnd so that it contains the object
type and a pointer to the object's parent object. A master-object type is
introduced, called pcb_any_obj_t - this contains only the header and can
represent any actual object.
This means instead of Type/Ptr1/Ptr2, it is enough to pass a single,
self-contained pcb_any_obj_t and the callee can figure what actual object
it is and who the parent is.
It's still far from true type safety^3, but at least it's much harder to
pass non-object pointers and it's impossible to pass incosistent data
(Type or Ptr1 not matching Ptr2). It also lets the debugger and the
compiler understand what we are doing.
We are in the early phase of this effort, and we are switching over the
code gradually. The first major part where Ptr1/Ptr2/Ptr3 started to
disappear is the netlist/rat (pin/pad lookup) code.
Footnotes:
^1: C is not strongly typed, so this mechanism saves us from runtime bugs
only to a certain extent. However, proper use of types and const can
really catch a lot of bugs compile time.
^2: http://repo.hu/projects/pcb-history/
^3: please DO NOT propose switching to C++ or sql or whatever your
favorite programming language is: pcb-rnd won't do that. If you want a
project that uses that language/tool, just fork pcb or pcb-rnd and do it.
- Raw text -