There are two questions here:
- Is const an effective safety mechanism?
If you use const in a way that allows the compiler to verify that
the invariants you want actually hold, and warn you when they don't,
then, yes. I'm all in favor of using const this way. If I've
missed such a case in the code, please point it out.
But if you use const to hint at what you mean --- say, by making
svn_fs__list_length's argument `const skel_t *', while
skel->children and skel->next are still mutable --- then it is not
an effective safety mechanism. If someone adds code which whacks a
skel node that was supposed to be left alone, the compiler probably
won't warn you. This half-hearted style of const usage doesn't give
you the "solid measure of safety" you're going for at all. To the
contrary, it's more like a thin layer of ice --- who knows whether
it can hold your weight?
- Should skels be read-only data structures?
I hadn't intended them to be. My thought with skel.[ch] was to lay
out a simple structure, describe a minimal set of invariants,
provide some utility functions which expect and preserve those
invariants, and beyond that, leave it up to the caller to use them
correctly.
I believe the current code does this. Skels don't fly around willy-
nilly, as you seem to think. The functions which build atoms from
literal strings are all in a position to ensure that the skel is
used correctly. The skel is immediately written to a database
record; it never passes outside the function, except to the skel
utility functions, whose behavior is well-known.
I think you're arguing that the entire tree of skel nodes should be
const --- that is, skel->data, skel->children, and skel->next would
all refer to const objects, and the code should be changed to use
constructor-style manipulation. I can see some real advantages to
this. You'd be shifting from the `hinting to the reader' style to
thea `compiler can check' style, which addresses the shortcomings
I've complained about before. The only casting necessary would be
in the functions that actually construct the skels to begin with,
which is reasonable.
However, it would make read / modify / write code, like that in
tree.c:txn_body_change_node_prop, impossible; it would become read /
reconstruct / write code. Perhaps this isn't a big loss.
In any case, this change should wait until after M2.
Setting the question of skel tree structure aside, I may agree with
you that skel->data should be const. I can't think of any situation
where it is appropriate to change *skel->data. Since the atom's text
is usually a substring in a larger object, you'd have to be dealing
with fixed-width data, which is pretty rare, and usually a bad idea to
begin with. This would clean up the string literal issue as well.
Received on Sat Oct 21 14:36:17 2006