[Verse-dev] Bone code broken?

[Emil Brink]

Hello.

I'm reading the host code for g_bone_create, and find the following
statements a wee bit suspicious (vs_node_geometry.c, lines 930 an on):

	for(i = 0; i < 16; i++)
		node->bones[bone_id].weight[i] = weight[i];
	node->bones[bone_id].weight[i] = 0;
	for(i = 0; i < 16; i++)
		node->bones[bone_id].reference[i] = reference[i];
	node->bones[bone_id].reference[i] = 0;

The corresponding definitions in the host code are:

typedef struct {
	char weight[16];
	char reference[16];
	[...]
} VSNGBone;

Now, I see at least four problems here:

1. The string (for these are strings, I'm fairly sure... layer names,
   to be specific) copying is done inline rather than with a function,
   making the code horrible to understand and maintain.
2. The loops don't terminate on NUL, they always copy a full 16 bytes.
3. The "16" is hardcoded, making for a needless dependency between the
   declaration and the code. This point is minor, the size changes very
   infrequently, but still.
4. The final NUL-write is done with i=16, which is *outside* the
   allocated structure fields. This doesn't in practice matter, but
   it's clearly wrong anyway.

I will fix 2) and 4), fixing 1) should be done all over the entire
code base and I'll leave that one to Eskil. 3) should be fixed in
parallel with 1), using sizeof in the copy call.

These are my opinions of course, and all based on a rather shallow
examination of the code (reading it, and trying a stand-alone loop
to verify claim 4).

Regards,

/Emil