Merge lp:~smspillaz/compiz-libcompizconfig/ccs-object into lp:~compiz-team/compiz-libcompizconfig/0.9.8

Proposed by Sam Spilsbury
Status: Superseded
Proposed branch: lp:~smspillaz/compiz-libcompizconfig/ccs-object
Merge into: lp:~compiz-team/compiz-libcompizconfig/0.9.8
Diff against target: 690 lines (+613/-1)
6 files modified
CMakeLists.txt (+1/-0)
include/ccs.h (+101/-1)
src/CMakeLists.txt (+3/-0)
src/main.c (+179/-0)
tests/CMakeLists.txt (+56/-0)
tests/test-ccs-object.cpp (+273/-0)
To merge this branch: bzr merge lp:~smspillaz/compiz-libcompizconfig/ccs-object
Reviewer Review Type Date Requested Status
Sam Spilsbury Needs Fixing
Alan Griffiths Pending
Review via email: mp+104864@code.launchpad.net

This proposal supersedes a proposal from 2012-05-04.

Description of the change

This is all about bug 990690.

!! - It probably isn't a good idea to test this branch in isolation, as it is part of a pipeline to get compiz-libcompizconfig under test. If you want to test the result of this work, you should probably look at testing

lp:~smspillaz/compiz-libcompizconfig/setting-mock
lp:~smspillaz/compiz-compizconfig-python/compiz-compizconfig-python.setting-api
lp:~smspillaz/compiz-compizconfig-gconf/compiz-compizconfig-gconf.adapt-to-new-interfaces

.. that's all !!

This branch introduces some preliminary work in a series of branches to get libcompizconfig under test. In order to test the objects properly, we need to abstract away their interfaces into replacable structs so you can test the interaction between the various classes.

This would be awkward to do correctly if we didn't have a suitable object system to handle interface implementation, referencing, private storage etc.

As such, a new struct CCSObject is introduced. It is similar in design to GObject, but with a much smaller feature set centered mostly around the handling of interfaces and composition / indirection. A type registrar is also included (also very simple).

Tests are included.

To post a comment you must log in.
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

I know it is common (and may be a project style) but there is no advantage to having different names to refer to the same thing in different namespaces. That is not:

    typedef struct _CCSObject CCSObject;

    struct _CCSObject
    {
    ...
    };

but:

    typedef struct CCSObject
    {
    ...
    } CCSObject;

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

> I know it is common (and may be a project style) but there is no advantage to
> having different names to refer to the same thing in different namespaces.
> That is not:
>
> typedef struct _CCSObject CCSObject;
>
> struct _CCSObject
> {
> ...
> };
>
> but:
>
> typedef struct CCSObject
> {
> ...
> } CCSObject;

Actually, it doesn't seem that client code should be interested in the members of [_]CCSObject, so why not a completely opaque type? Vis:

    typedef struct CCSObject CCSObject;

in the header. And move the definition to the .c file.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

I don't like the style of coding in ccsObjectAddInterface() - if either realloc fails then the object is hosed. Admittedly FALSE is returned to inform the caller, but I'd prefer the object to remain viable and just the call to fail. Anything else can cause nasty surprises in client code. (Admittedly, if realloc fails there are probably worse problems to deal with.)

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

> I don't like the style of coding in ccsObjectAddInterface() - if either
> realloc fails then the object is hosed. Admittedly FALSE is returned to
> inform the caller, but I'd prefer the object to remain viable and just the
> call to fail. Anything else can cause nasty surprises in client code.
> (Admittedly, if realloc fails there are probably worse problems to deal with.)

Forgot to mention: the cost of calling realloc in ccsObjectRemoveInterface probably outweighs any saving of memory that results.

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> > I know it is common (and may be a project style) but there is no advantage
> to
> > having different names to refer to the same thing in different namespaces.
> > That is not:
> >
> > typedef struct _CCSObject CCSObject;
> >
> > struct _CCSObject
> > {
> > ...
> > };
> >
> > but:
> >
> > typedef struct CCSObject
> > {
> > ...
> > } CCSObject;
>
> Actually, it doesn't seem that client code should be interested in the members
> of [_]CCSObject, so why not a completely opaque type? Vis:
>
> typedef struct CCSObject CCSObject;
>
> in the header. And move the definition to the .c file.

I think the struct can't be opaque because of the way that we actually access the object data (eg, casting it to a CCSObject * and accessing the data in the CCSObject in the first part of the struct).

As for realloc - I agree and had similar concerns, and will change that.

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

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.

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

Let me suggest a way that the client code could be eg "ccsObjectRef(to);"...
...
Bool ccsObjectRef_(CCSObject *object);
#define ccsObjectRef(obj) ccsObjectRef_(&obj->object)
...
No nasty casts, and type safe. At the cost of requiring a member named "object" of CCSObject type.

Revision history for this message
Sam Spilsbury (smspillaz) wrote : Posted in a previous version of this proposal

> 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?

Indeed, future merges will introduce proper constructors (but this is not trivial right now).

>
> 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.

They use realloc, so I don't see why this is a problem ?

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

ack.

>
> 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.)

Right, we should evaluate this elsewhere

>
> 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.

Right, this sucks, but we're introspecting the state of the object. Mabye we can get rid of the casts and just do to->object.foo

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

> > 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.
>
> They use realloc, so I don't see why this is a problem ?

Ah, I wrongly assumed you were using a function with different semantics to avoid the problems discussed earlier (i.e. invalidating the object on failed realloc).

Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

ccsObjectRemoveInterface_ doesn't use realloc, so it just overwrites the pointer. I think that will leak.

ccsObjectAddInterface does use realloc (I was wrongly assuming that you'd addressed the earlier discussion of invalidating the object on failed operation). Invalidating the object and returning FALSE will cause nasty surprises in client code.

Is there really value in CCSObjectAllocationInterface?

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

These macros can lead to surprising results in user code:

86 +#define ccsObjectRef(o) \
87 + ((o)->object).refcnt++;
88 +
89 +#define ccsObjectUnref(o, freeFunc) \
90 + ((o)->object).refcnt--; \
91 + if (!((o)->object).refcnt) \
92 + freeFunc (o)

Example of problem:

  if (false) ccsObjectUnref(o, f);

will call f(o).

Rewrite as:
86 +#define ccsObjectRef(o) \
87 + do { ((o)->object).refcnt++ } while(false)
88 +
89 +#define ccsObjectUnref(o, freeFunc) \
90 + do { ((o)->object).refcnt--; \
91 + if (!((o)->object).refcnt) \
92 + freeFunc (o) } while(false)

PS It feels awkward to require freeFunc as a parameter, but I don't have a cleaner alternative suggestion just now.

review: Needs Fixing
Revision history for this message
Alan Griffiths (alan-griffiths) wrote : Posted in a previous version of this proposal

Bad example.

  if (false) ccsObjectUnref(o, f);

is OK.

But...

  if (true) ccsObjectUnref(o, f);
  else printf("surprise");

is a good example (i.e. broken).

437. By Sam Spilsbury

Free interfaces once objects are removed

438. By Sam Spilsbury

Fix potential macro problems

Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Keeping old status

review: Needs Fixing
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

>ccsObjectRemoveInterface_ doesn't use realloc, so it just overwrites the pointer. I think that will leak.

Fixed as discussed

> ccsObjectAddInterface does use realloc (I was wrongly assuming that you'd addressed the earlier discussion of invalidating the object on failed operation). Invalidating the object and returning FALSE will cause nasty surprises in client code.

Not an issue as discussed on irc. (realloc (foo, sizeof (whatever)); doesn't invalidate foo if realloc fails)

> Is there really value in CCSObjectAllocationInterface?

Yes, for mocking allocation failures. Feel crap having to inject it, but there isn't much of an alternative

> Bad example.

> if (false) ccsObjectUnref(o, f);

> is OK.

> But...

> if (true) ccsObjectUnref(o, f);
> else printf("surprise");

> is a good example (i.e. broken).

Fixed as discussed

> PS It feels awkward to require freeFunc as a parameter, but I don't have a cleaner alternative suggestion just now.

If / when we add proper constructor / destructor semantics this will go away :) (That's a larger change though that should be done later)

439. By Sam Spilsbury

Lose the ;

440. By Sam Spilsbury

Added a free thread

441. By Sam Spilsbury

Also test for free

Unmerged revisions

441. By Sam Spilsbury

Also test for free

440. By Sam Spilsbury

Added a free thread

439. By Sam Spilsbury

Lose the ;

438. By Sam Spilsbury

Fix potential macro problems

437. By Sam Spilsbury

Free interfaces once objects are removed

436. By Sam Spilsbury

Don't use reserved name

435. By Sam Spilsbury

Make the CCSObject API a bit more typesafe

434. By Sam Spilsbury

Don't reallocate all the time. Also handle realloc failures better

433. By Sam Spilsbury

Added a simple type registrar

432. By Sam Spilsbury

Added a ccsObjectFinalize function which frees everything in CCSObject storage

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'CMakeLists.txt'
--- CMakeLists.txt 2011-07-07 12:23:21 +0000
+++ CMakeLists.txt 2012-05-07 10:49:18 +0000
@@ -141,6 +141,7 @@
141add_subdirectory (src)141add_subdirectory (src)
142add_subdirectory (include)142add_subdirectory (include)
143add_subdirectory (cmake)143add_subdirectory (cmake)
144add_subdirectory (tests)
144145
145compiz_print_configure_header ("CompizConfig Library")146compiz_print_configure_header ("CompizConfig Library")
146compiz_color_message ("\n${_escape}[4mOptional features:${_escape}[0m\n")147compiz_color_message ("\n${_escape}[4mOptional features:${_escape}[0m\n")
147148
=== modified file 'include/ccs.h'
--- include/ccs.h 2011-08-20 19:03:37 +0000
+++ include/ccs.h 2012-05-07 10:49:18 +0000
@@ -19,10 +19,14 @@
19 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA19 * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA
20 */20 */
2121
22
23#ifndef _CSS_H22#ifndef _CSS_H
24#define _CSS_H23#define _CSS_H
2524
25#ifdef __cplusplus
26extern "C"
27{
28#endif
29
26#define D_NONE 030#define D_NONE 0
27#define D_NORMAL 131#define D_NORMAL 1
28#define D_FULL 232#define D_FULL 2
@@ -124,6 +128,98 @@
124CCSLIST_HDR (StrRestriction, CCSStrRestriction)128CCSLIST_HDR (StrRestriction, CCSStrRestriction)
125CCSLIST_HDR (StrExtension, CCSStrExtension)129CCSLIST_HDR (StrExtension, CCSStrExtension)
126130
131typedef struct _CCSInterface CCSInterface; /* Dummy typedef */
132typedef struct _CCSPrivate CCSPrivate; /* Dummy typedef */
133typedef struct _CCSObject CCSObject;
134
135typedef void * (*reallocProc) (void *, size_t);
136typedef void * (*mallocProc) (size_t);
137typedef void * (*callocProc) (size_t, size_t);
138
139typedef struct _CCSObjectAllocationInterface
140{
141 reallocProc realloc_;
142 mallocProc malloc_;
143 callocProc calloc_;
144} CCSObjectAllocationInterface;
145
146extern CCSObjectAllocationInterface ccsDefaultObjectAllocator;
147
148struct _CCSObject
149{
150 CCSPrivate *priv; /* Private pointer for object storage */
151
152 const CCSInterface **interfaces; /* An array of interfaces that this object implements */
153 int *interface_types; /* An array of interface types */
154 unsigned int n_interfaces;
155 unsigned int n_allocated_interfaces;
156
157 CCSObjectAllocationInterface *object_allocation;
158
159 unsigned int refcnt; /* Reference count of this object */
160};
161
162Bool
163ccsObjectInit_ (CCSObject *object, CCSObjectAllocationInterface *interface);
164
165#define ccsObjectInit(o, interface) (ccsObjectInit_) (&(o)->object, interface)
166
167Bool
168ccsObjectAddInterface_ (CCSObject *object, const CCSInterface *interface, int interface_type);
169
170#define ccsObjectAddInterface(o, interface, type) (ccsObjectAddInterface_) (&(o)->object, interface, type);
171
172Bool
173ccsObjectRemoveInterface_ (CCSObject *object, int interface_type);
174
175#define ccsObjectRemoveInterface(o, interface_type) (ccsObjectRemoveInterface_) (&(o)->object, interface_type);
176
177const CCSInterface * ccsObjectGetInterface_ (CCSObject *object, int interface_type);
178
179#define ccsObjectGetInterface(o, interface_type) (ccsObjectGetInterface_) (&(o)->object, interface_type)
180
181#define ccsObjectRef(o) \
182 do { ((o)->object).refcnt++; } while (FALSE)
183
184#define ccsObjectUnref(o, freeFunc) \
185 do \
186 { \
187 ((o)->object).refcnt--; \
188 if (!((o)->object).refcnt) \
189 freeFunc (o); \
190 } while (FALSE)
191
192CCSPrivate *
193ccsObjectGetPrivate_ (CCSObject *object);
194
195#define ccsObjectGetPrivate(o) (ccsObjectGetPrivate_) (&(o)->object)
196
197void
198ccsObjectSetPrivate_ (CCSObject *object, CCSPrivate *priv);
199
200#define ccsObjectSetPrivate(o, priv) (ccsObjectSetPrivate_) (&(o)->object, priv)
201
202void
203ccsObjectFinalize_ (CCSObject *object);
204
205#define ccsObjectFinalize(o) (ccsObjectFinalize_) (&(o)->object)
206
207unsigned int
208ccsAllocateType ();
209
210#define GET_INTERFACE_TYPE(Interface) \
211 ccs##Interface##GetType ();
212
213#define INTERFACE_TYPE(Interface) \
214 inline unsigned int ccs##Interface##GetType () \
215 { \
216 static unsigned int type_id = 0; \
217 if (!type_id) \
218 type_id = ccsAllocateType (); \
219 \
220 return type_id; \
221 }
222
127/**223/**
128 * reference counting224 * reference counting
129 * 225 *
@@ -913,4 +1009,8 @@
913/* Checks if settings need to be constrained */1009/* Checks if settings need to be constrained */
914Bool ccsContextNeedsConstraining (CCSContext *context);1010Bool ccsContextNeedsConstraining (CCSContext *context);
9151011
1012#ifdef __cplusplus
1013}
1014#endif
1015
916#endif1016#endif
9171017
=== modified file 'src/CMakeLists.txt'
--- src/CMakeLists.txt 2011-10-15 07:28:45 +0000
+++ src/CMakeLists.txt 2012-05-07 10:49:18 +0000
@@ -35,6 +35,9 @@
35 ${LIBCOMPIZCONFIG_FILES}35 ${LIBCOMPIZCONFIG_FILES}
36 compizconfig.pb.cc36 compizconfig.pb.cc
37 )37 )
38 set (LIBCOMPIZCONFIG_LIBRARIES
39 ${LIBCOMPIZCONFIG_LIBRARIES}
40 protobuf)
38 add_definitions (41 add_definitions (
39 -DUSE_PROTOBUF42 -DUSE_PROTOBUF
40 )43 )
4144
=== modified file 'src/main.c'
--- src/main.c 2011-11-10 00:58:18 +0000
+++ src/main.c 2012-05-07 10:49:18 +0000
@@ -39,6 +39,173 @@
39#include "ccs-private.h"39#include "ccs-private.h"
40#include "iniparser.h"40#include "iniparser.h"
4141
42CCSObjectAllocationInterface ccsDefaultObjectAllocator =
43{
44 realloc,
45 malloc,
46 calloc
47};
48
49/* CCSObject stuff */
50Bool
51ccsObjectInit_(CCSObject *object, CCSObjectAllocationInterface *object_allocation)
52{
53 object->priv = NULL;
54 object->n_interfaces = 0;
55 object->n_allocated_interfaces = 0;
56 object->interfaces = NULL;
57 object->interface_types = NULL;
58 object->object_allocation = object_allocation;
59 object->refcnt = 0;
60
61 return TRUE;
62}
63
64Bool
65ccsObjectAddInterface_(CCSObject *object, const CCSInterface *interface, int interface_type)
66{
67 object->n_interfaces++;
68
69 if (object->n_allocated_interfaces < object->n_interfaces)
70 {
71 unsigned int old_allocated_interfaces = object->n_allocated_interfaces;
72 object->n_allocated_interfaces = object->n_interfaces;
73 CCSInterface **ifaces = (*object->object_allocation->realloc_) (object->interfaces, object->n_allocated_interfaces * sizeof (CCSInterface *));
74 int *iface_types = (*object->object_allocation->realloc_) (object->interface_types, object->n_allocated_interfaces * sizeof (int));
75
76 if (!ifaces || !iface_types)
77 {
78 if (ifaces)
79 free (ifaces);
80
81 if (iface_types)
82 free (iface_types);
83
84 object->n_interfaces--;
85 object->n_allocated_interfaces = old_allocated_interfaces;
86 return FALSE;
87 }
88 else
89 {
90 object->interfaces = (const CCSInterface **) ifaces;
91 object->interface_types = iface_types;
92 }
93 }
94
95 object->interfaces[object->n_interfaces - 1] = interface;
96 object->interface_types[object->n_interfaces - 1] = interface_type;
97
98 return TRUE;
99}
100
101Bool
102ccsObjectRemoveInterface_(CCSObject *object, int interface_type)
103{
104 unsigned int i = 0;
105
106 if (!object->n_interfaces)
107 return FALSE;
108
109 const CCSInterface **o = object->interfaces;
110 int *type = object->interface_types;
111
112 for (; i < object->n_interfaces; i++, o++, type++)
113 {
114 if (object->interface_types[i] == interface_type)
115 break;
116 }
117
118 if (i >= object->n_interfaces)
119 return FALSE;
120
121 /* Now clear this section and move everything back */
122 object->interfaces[i] = NULL;
123
124 i++;
125
126 const CCSInterface **oLast = o;
127 int *typeLast = type;
128
129 o++;
130 type++;
131
132 memmove ((void *) oLast, (void *)o, (object->n_interfaces - i) * sizeof (CCSInterface *));
133 memmove ((void *) typeLast, (void *) type, (object->n_interfaces - i) * sizeof (int));
134
135 object->n_interfaces--;
136
137 if (!object->n_interfaces)
138 {
139 free (object->interfaces);
140 free (object->interface_types);
141 object->interfaces = NULL;
142 object->interface_types = NULL;
143 object->n_allocated_interfaces = 0;
144 }
145
146 return TRUE;
147}
148
149const CCSInterface *
150ccsObjectGetInterface_(CCSObject *object, int interface_type)
151{
152 unsigned int i = 0;
153
154 for (; i < object->n_interfaces; i++)
155 {
156 if (object->interface_types[i] == interface_type)
157 return object->interfaces[i];
158 }
159
160 return NULL;
161}
162
163CCSPrivate *
164ccsObjectGetPrivate_(CCSObject *object)
165{
166 return object->priv;
167}
168
169void
170ccsObjectSetPrivate_(CCSObject *object, CCSPrivate *priv)
171{
172 object->priv = priv;
173}
174
175void
176ccsObjectFinalize_(CCSObject *object)
177{
178 if (object->priv)
179 {
180 free (object->priv);
181 object->priv = NULL;
182 }
183
184 if (object->interfaces)
185 {
186 free (object->interfaces);
187 object->interfaces = NULL;
188 }
189
190 if (object->interface_types)
191 {
192 free (object->interface_types);
193 object->interface_types = NULL;
194 }
195
196 object->n_interfaces = 0;
197}
198
199unsigned int
200ccsAllocateType ()
201{
202 static unsigned int start = 0;
203
204 start++;
205
206 return start;
207}
208
42Bool basicMetadata = FALSE;209Bool basicMetadata = FALSE;
43210
44void211void
@@ -592,6 +759,18 @@
592CCSREF (StrRestriction, CCSStrRestriction)759CCSREF (StrRestriction, CCSStrRestriction)
593CCSREF (StrExtension, CCSStrExtension)760CCSREF (StrExtension, CCSStrExtension)
594761
762#define CCSREF_OBJ(type,dtype) \
763 void ccs##type##Ref (dtype *d) \
764 { \
765 ((CCSObject *) d)->refcnt++; \
766 } \
767 void ccs##type##Unref (dtype *d) \
768 { \
769 ((CCSObject *) d)->refcnt--; \
770 if (((CCSObject *) d)->refcnt == 0) \
771 ccsFree##type (d); \
772 } \
773
595static void *774static void *
596openBackend (char *backend)775openBackend (char *backend)
597{776{
598777
=== added directory 'tests'
=== added file 'tests/CMakeLists.txt'
--- tests/CMakeLists.txt 1970-01-01 00:00:00 +0000
+++ tests/CMakeLists.txt 2012-05-07 10:49:18 +0000
@@ -0,0 +1,56 @@
1# Build Google Test and make its headers known
2find_package (GTest)
3
4if (NOT GTEST_FOUND)
5
6 # Check for google test and build it locally
7 set (GTEST_ROOT_DIR
8 "/usr/src/gtest" # Default value, adjustable by user with e.g., ccmake
9 CACHE
10 PATH
11 "Path to Google test srcs"
12 )
13
14 find_path (GTEST_INCLUDE_DIR gtest/gtest.h)
15
16 if (GTEST_INCLUDE_DIR)
17 #FIXME - hardcoded is bad!
18 add_subdirectory (${GTEST_ROOT_DIR}
19 gtest)
20 endif(GTEST_INCLUDE_DIR)
21
22 set (GTEST_BOTH_LIBRARIES gtest gtest_main)
23 set (GTEST_FOUND TRUE)
24
25endif (NOT GTEST_FOUND)
26
27find_library (GMOCK_LIBRARY gmock)
28find_library (GMOCK_MAIN_LIBRARY gmock_main)
29
30if (NOT GMOCK_LIBRARY OR NOT GMOCK_MAIN_LIBRARY OR NOT GTEST_FOUND)
31 message ("Google Mock and Google Test not found - cannot build tests!")
32 set (COMPIZ_BUILD_TESTING OFF)
33endif (NOT GMOCK_LIBRARY OR NOT GMOCK_MAIN_LIBRARY OR NOT GTEST_FOUND)
34
35include_directories (${GTEST_INCLUDE_DIRS})
36include_directories (${CMAKE_SOURCE_DIR}/include)
37link_directories (${CMAKE_INSTALL_PREFIX}/lib)
38
39add_executable (test-ccs-object
40 ${CMAKE_CURRENT_SOURCE_DIR}/test-ccs-object.cpp)
41
42if (HAVE_PROTOBUF)
43 set (LIBCOMPIZCONFIG_LIBRARIES
44 ${LIBCOMPIZCONFIG_LIBRARIES}
45 protobuf)
46endif (HAVE_PROTOBUF)
47
48target_link_libraries (test-ccs-object
49 ${GTEST_BOTH_LIBRARIES}
50 ${GMOCK_LIBRARY}
51 ${GMOCK_MAIN_LIBRARY}
52 ${CMAKE_THREAD_LIBS_INIT}
53 ${LIBCOMPIZCONFIG_LIBRARIES}
54 compizconfig)
55
56gtest_add_tests (test-ccs-object "" ${CMAKE_CURRENT_SOURCE_DIR}/test-ccs-object.cpp)
057
=== added file 'tests/test-ccs-object.cpp'
--- tests/test-ccs-object.cpp 1970-01-01 00:00:00 +0000
+++ tests/test-ccs-object.cpp 2012-05-07 10:49:18 +0000
@@ -0,0 +1,273 @@
1#include <gtest/gtest.h>
2#include <gmock/gmock.h>
3#include <ccs.h>
4#include <cstdio>
5
6using ::testing::_;
7using ::testing::Return;
8
9class CCSObjectTest :
10 public ::testing::Test
11{
12};
13
14struct TestingObjectWrapper
15{
16 CCSObject object;
17};
18
19typedef void (*dummyFunc) (void);
20
21struct DummyInterface
22{
23 dummyFunc dummy;
24};
25
26struct Dummy2Interface
27{
28 dummyFunc dummy;
29};
30
31class GoogleMockDummyInterface
32{
33 public:
34
35 virtual ~GoogleMockDummyInterface () {};
36
37 virtual void dummyFunc () = 0;
38 virtual void freeTestingObjectWrapper (TestingObjectWrapper *) = 0;
39};
40
41class GoogleMockDummy :
42 public GoogleMockDummyInterface
43{
44 public:
45
46 MOCK_METHOD0 (dummyFunc, void ());
47 MOCK_METHOD1 (freeTestingObjectWrapper, void (TestingObjectWrapper *));
48 public:
49
50 static void thunkDummyFunc () { return _mockDummy.dummyFunc (); }
51 static GoogleMockDummy _mockDummy;
52};
53
54GoogleMockDummy GoogleMockDummy::_mockDummy;
55
56const struct DummyInterface SomeDummyInterface =
57{
58 GoogleMockDummy::thunkDummyFunc
59};
60
61#define CCS_INTERFACE_TYPE_DUMMY GET_INTERFACE_TYPE (DummyInterface)
62#define CCS_INTERFACE_TYPE_DUMMY2 GET_INTERFACE_TYPE (Dummy2Interface)
63
64INTERFACE_TYPE (DummyInterface)
65INTERFACE_TYPE (Dummy2Interface)
66
67TEST(CCSObjectTest, TestTypeAllocation)
68{
69 unsigned int i = CCS_INTERFACE_TYPE_DUMMY;
70 unsigned int j = CCS_INTERFACE_TYPE_DUMMY;
71 unsigned int k = CCS_INTERFACE_TYPE_DUMMY2;
72
73 EXPECT_EQ (i, 1);
74 EXPECT_EQ (j ,1);
75 EXPECT_EQ (k, 2);
76}
77
78TEST(CCSObjectTest, InterfaceAdd)
79{
80 TestingObjectWrapper *to = (TestingObjectWrapper *) calloc (1, sizeof (TestingObjectWrapper));
81
82 ccsObjectInit (to, &ccsDefaultObjectAllocator);
83 ccsObjectAddInterface (to, (const CCSInterface *) &SomeDummyInterface, 1);
84
85 EXPECT_EQ (*((CCSObject *) to)->interfaces, (const CCSInterface *) (&SomeDummyInterface));
86 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 1);
87 EXPECT_EQ (*((CCSObject *) to)->interface_types, 1);
88
89 free (to);
90}
91
92TEST(CCSObjectTest, InterfaceRemove)
93{
94 TestingObjectWrapper *to = (TestingObjectWrapper *) calloc (1, sizeof (TestingObjectWrapper));
95
96 ccsObjectInit (to, &ccsDefaultObjectAllocator);
97 ccsObjectAddInterface (to, (const CCSInterface *) &SomeDummyInterface, 1);
98
99 EXPECT_EQ (*((CCSObject *) to)->interfaces, (const CCSInterface *) (&SomeDummyInterface));
100 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 1);
101 EXPECT_EQ (*((CCSObject *) to)->interface_types, 1);
102
103 ccsObjectRemoveInterface (to, 1);
104
105 EXPECT_EQ (NULL, ((CCSObject *) to)->interfaces);
106 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 0);
107 EXPECT_EQ (NULL, ((CCSObject *) to)->interface_types);
108
109 free (to);
110}
111
112TEST(CCSObjectTest, InterfaceFetchCall)
113{
114 TestingObjectWrapper *to = (TestingObjectWrapper *) calloc (1, sizeof (TestingObjectWrapper));
115
116 ccsObjectInit (to, &ccsDefaultObjectAllocator);
117 ccsObjectAddInterface (to, (const CCSInterface *) &SomeDummyInterface, 1);
118
119 EXPECT_EQ (*((CCSObject *) to)->interfaces, (const CCSInterface *) (&SomeDummyInterface));
120 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 1);
121 EXPECT_EQ (*((CCSObject *) to)->interface_types, 1);
122
123 const DummyInterface *myDummyInterface = (const DummyInterface *) ccsObjectGetInterface (to, 1);
124
125 EXPECT_CALL (GoogleMockDummy::_mockDummy, dummyFunc ());
126
127 (*myDummyInterface->dummy) ();
128
129 ccsObjectRemoveInterface (to, 1);
130
131 EXPECT_EQ (NULL, ((CCSObject *) to)->interfaces);
132 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 0);
133 EXPECT_EQ (NULL, ((CCSObject *) to)->interface_types);
134
135 free (to);
136}
137
138TEST(CCSObjectTest, SetPrivateGetPrivate)
139{
140 TestingObjectWrapper *to = (TestingObjectWrapper *) calloc (1, sizeof (TestingObjectWrapper));
141
142 int i = 1;
143
144 ccsObjectInit (to, &ccsDefaultObjectAllocator);
145 ccsObjectSetPrivate (to, (CCSPrivate *) &i);
146
147 CCSPrivate *p = ccsObjectGetPrivate (to);
148
149 EXPECT_EQ (&i, (int *) p);
150 EXPECT_EQ (i, (*((int *) p)));
151}
152
153void ccsFreeTestingObjectWrapper (TestingObjectWrapper *wrapper)
154{
155 GoogleMockDummy::_mockDummy.freeTestingObjectWrapper (wrapper);
156}
157
158#define CCSREF_OBJ(type,dtype) \
159 void ccs##type##Ref (dtype *d) \
160 { \
161 ccsObjectRef (d); \
162 } \
163 \
164 void ccs##type##Unref (dtype *d) \
165 { \
166 ccsObjectUnref (d, ccsFree##type); \
167 } \
168
169CCSREF_HDR(TestingObjectWrapper, TestingObjectWrapper);
170CCSREF_OBJ(TestingObjectWrapper, TestingObjectWrapper);
171
172TEST(CCSObjectTest, TestRefUnrefFreesObject)
173{
174 TestingObjectWrapper *to = (TestingObjectWrapper *) calloc (1, sizeof (TestingObjectWrapper));
175
176 ccsObjectInit (to, &ccsDefaultObjectAllocator);
177 ccsTestingObjectWrapperRef (to);
178
179 EXPECT_CALL (GoogleMockDummy::_mockDummy, freeTestingObjectWrapper (to));
180
181 ccsTestingObjectWrapperUnref (to);
182}
183
184TEST(CCSObjectTest, TestFinalizeFreesEverything)
185{
186 TestingObjectWrapper *to = (TestingObjectWrapper *) calloc (1, sizeof (TestingObjectWrapper));
187
188 ccsObjectInit (to, &ccsDefaultObjectAllocator);
189 ccsObjectAddInterface (to, (const CCSInterface *) &SomeDummyInterface, 1);
190
191 EXPECT_EQ (*((CCSObject *) to)->interfaces, (const CCSInterface *) (&SomeDummyInterface));
192 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 1);
193 EXPECT_EQ (*((CCSObject *) to)->interface_types, 1);
194
195 int *i = (int *) malloc (sizeof (int));
196
197 *i = 1;
198
199 ccsObjectSetPrivate (to, (CCSPrivate *) i);
200
201 CCSPrivate *p = ccsObjectGetPrivate (to);
202
203 EXPECT_EQ (i, (int *) p);
204 EXPECT_EQ (*i, (*((int *) p)));
205
206 ccsObjectFinalize (to);
207
208 EXPECT_EQ (NULL, ((CCSObject *) to)->priv);
209 EXPECT_EQ (NULL, ((CCSObject *) to)->interfaces);
210 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 0);
211 EXPECT_EQ (NULL, ((CCSObject *) to)->interface_types);
212}
213
214void *
215failingRealloc (void *p, size_t n)
216{
217 return NULL;
218}
219
220CCSObjectAllocationInterface failingAllocator =
221{
222 failingRealloc,
223 malloc,
224 calloc
225};
226
227TEST(CCSObjectTest, TestReallocFailures)
228{
229 TestingObjectWrapper *to = (TestingObjectWrapper *) calloc (1, sizeof (TestingObjectWrapper));
230
231 ccsObjectInit (to, &failingAllocator);
232 ccsObjectAddInterface (to, (const CCSInterface *) &SomeDummyInterface, 1);
233
234 EXPECT_EQ (NULL, ((CCSObject *) to)->interfaces);
235 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 0);
236 EXPECT_EQ (NULL, ((CCSObject *) to)->interface_types);
237
238 free (to);
239}
240
241TEST(CCSObjectTest, TestAllocatedMemoryOnRemove)
242{
243 TestingObjectWrapper *to = (TestingObjectWrapper *) calloc (1, sizeof (TestingObjectWrapper));
244
245 ccsObjectInit (to, &ccsDefaultObjectAllocator);
246 ccsObjectAddInterface (to, (const CCSInterface *) &SomeDummyInterface, 1);
247
248 EXPECT_EQ (*((CCSObject *) to)->interfaces, (const CCSInterface *) (&SomeDummyInterface));
249 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 1);
250 EXPECT_EQ (*((CCSObject *) to)->interface_types, 1);
251
252 ccsObjectAddInterface (to, (const CCSInterface *) &SomeDummyInterface, 2);
253
254 EXPECT_EQ (*((CCSObject *) to)->interfaces, (const CCSInterface *) (&SomeDummyInterface));
255 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 2);
256 EXPECT_EQ (*((CCSObject *) to)->interface_types, 1);
257
258 ccsObjectRemoveInterface (to, 1);
259
260 EXPECT_EQ (*((CCSObject *) to)->interfaces, (const CCSInterface *) (&SomeDummyInterface));
261 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 1);
262 EXPECT_EQ (((CCSObject *) to)->n_allocated_interfaces, 2);
263 EXPECT_EQ (*((CCSObject *) to)->interface_types, 2);
264
265 ccsObjectRemoveInterface (to, 2);
266
267 EXPECT_EQ (NULL, ((CCSObject *) to)->interfaces);
268 EXPECT_EQ (((CCSObject *) to)->n_interfaces, 0);
269 EXPECT_EQ (((CCSObject *) to)->n_allocated_interfaces, 0);
270 EXPECT_EQ (NULL, ((CCSObject *) to)->interface_types);
271
272 free (to);
273}

Subscribers

People subscribed via source and target branches