Code review comment for lp://staging/~smspillaz/compiz-libcompizconfig/ccs-object

ccsObjectInit is only used by the test harness which ignores the return code, so the intended use is unclear. Surely a better return value would be a pointer to the initialised object?

ccsObjectRemoveInterface assigns pointers "interfaces" and "interface_types" to NULL without first freeing the memory referenced.

ccsObjectAddInterface assigns pointers "interfaces" and "interface_types" without first freeing the memory referenced.

struct _CCSObjectAllocationInterface (again, no need for the "_") has members realloc, malloc, calloc - these are reserved names.

Actually, talking of reserved names: names that begin with an underscore and a capital are reserved, which is another reason for not using these names in the struct namespace. (I still see no point in having different names (e.g. _CCSObject) in the struct namespace.)

The tests imply that the intended usage is to cast a pointer to some other structure (represented by "TestingObjectWrapper") to CCSObject* before passing that to ccsObjectXXX. If this is not the intended usage the tests are inappropriate. If it is intended there's negative value in these taking a pointer to the CCSObject as a parameter, they may as well take void* in the same way as ccsObjectInit does. It's a waste of the type-safety features of C.