Merge lp:~compiz-team/compiz/compiz.fix_1058577 into lp:compiz/0.9.9

[Sam Spilsbury]

Added tests for the StringToModifiers and ModifiersToString code, move the
code which detects <Primary> as ControlMask upstream and use two functions to
ensure that duplicates aren't added.

Fixes Jenkins failures. (LP: #1058577)

Added tests for the StringToModifiers and ModifiersToString code, move the code which detects <Primary> as ControlMask upstream and use two functions to ensure that duplicates aren't added.

Two tests were failing in bug 1058577 because both <Control> and <Primary> were added to the modifier string instead of just <Control>.

[Sam Spilsbury (smspillaz)]

Note, jenkins will probably mark this needs fixing because of conflicts with a distro patch. That distro patch should be dropped as its effectively made redundant by this branch.

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:3407
http://jenkins.qa.ubuntu.com/job/compiz-ci/149/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/149/console

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Sorry, but gcc sucks at template error messages:

[ 40%] Building CXX object compizconfig/libcompizconfig/tests/CMakeFiles/compizconfig_test_ccs_util.dir/compizconfig_test_ccs_util.cpp.o
In file included from /usr/include/c++/4.6/vector:70:0,
                 from /usr/include/gtest/gtest.h:55,
                 from /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp:22:
/usr/include/c++/4.6/bits/vector.tcc: In member function ‘void std::vector<_Tp, _Alloc>::_M_insert_aux(std::vector<_Tp, _Alloc>::iterator, _Args&& ...) [with _Args = {{anonymous}::ModifierParam}, _Tp = {anonymous}::ModifierParam, _Alloc = std::allocator<{anonymous}::ModifierParam>, std::vector<_Tp, _Alloc>::iterator = __gnu_cxx::__normal_iterator<{anonymous}::ModifierParam*, std::vector<{anonymous}::ModifierParam> >, typename std::_Vector_base<_Tp, _Alloc>::_Tp_alloc_type::pointer = {anonymous}::ModifierParam*]’:
/usr/include/c++/4.6/bits/vector.tcc:102:4: instantiated from ‘void std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args = {{anonymous}::ModifierParam}, _Tp = {anonymous}::ModifierParam, _Alloc = std::allocator<{anonymous}::ModifierParam>]’
/usr/include/c++/4.6/bits/stl_vector.h:840:9: instantiated from ‘void std::vector<_Tp, _Alloc>::push_back(std::vector<_Tp, _Alloc>::value_type&&) [with _Tp = {anonymous}::ModifierParam, _Alloc = std::allocator<{anonymous}::ModifierParam>, std::vector<_Tp, _Alloc>::value_type = {anonymous}::ModifierParam]’
/home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp:131:22: instantiated from here
/usr/include/c++/4.6/bits/vector.tcc:319:4: error: use of deleted function ‘{anonymous}::ModifierParam& {anonymous}::ModifierParam::operator=(const {anonymous}::ModifierParam&)’
/home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp:94:11: error: ‘{anonymous}::ModifierParam& {anonymous}::ModifierParam::operator=(const {anonymous}::ModifierParam&)’ is implicitly deleted because the default definition would be ill-formed:
/home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp:94:11: error: passing ‘const string {aka const std::basic_string<char>}’ as ‘this’ argument of ‘std::basic_string<_CharT, _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::operator=(const std::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char, _Traits = std::char_traits<char>, _Alloc = std::allocator<char>, std::basic_string<_CharT, _Traits, _Alloc> = std::basic_string<char>]’ discards qualifiers [-fpermissive]
In file included from /usr/include/c++/4.6/vector:61:0,
                 from /usr/include/gtest/gtest.h:55,
                 from /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp:22:
/usr/include/c++/4.6/bits/stl_algobase.h: In static member function ‘static _BI2 std::__copy_move_backward<true, false, std::random_access_iterator_tag>::__copy_move_b(_BI1, _BI1, _BI2) [with _BI1 = {anonymous}::ModifierParam*, _BI2 = {anonymous}::ModifierParam*]’:
/usr/include/c++/4.6/bits/stl_algobase.h:581:18: instantiated from ‘_BI2 std::_...

review: Needs Fixing

[Sam Spilsbury (smspillaz)]

> Sorry, but gcc sucks at template error messages:
>
> [ 40%] Building CXX object compizconfig/libcompizconfig/tests/CMakeFiles/compi
> zconfig_test_ccs_util.dir/compizconfig_test_ccs_util.cpp.o
> In file included from /usr/include/c++/4.6/vector:70:0,
> from /usr/include/gtest/gtest.h:55,
> from /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfi
> g/tests/compizconfig_test_ccs_util.cpp:22:
> /usr/include/c++/4.6/bits/vector.tcc: In member function ‘void
> std::vector<_Tp, _Alloc>::_M_insert_aux(std::vector<_Tp, _Alloc>::iterator,
> _Args&& ...) [with _Args = {{anonymous}::ModifierParam}, _Tp =
> {anonymous}::ModifierParam, _Alloc =
> std::allocator<{anonymous}::ModifierParam>, std::vector<_Tp, _Alloc>::iterator
> = __gnu_cxx::__normal_iterator<{anonymous}::ModifierParam*,
> std::vector<{anonymous}::ModifierParam> >, typename std::_Vector_base<_Tp,
> _Alloc>::_Tp_alloc_type::pointer = {anonymous}::ModifierParam*]’:
> /usr/include/c++/4.6/bits/vector.tcc:102:4: instantiated from ‘void
> std::vector<_Tp, _Alloc>::emplace_back(_Args&& ...) [with _Args =
> {{anonymous}::ModifierParam}, _Tp = {anonymous}::ModifierParam, _Alloc =
> std::allocator<{anonymous}::ModifierParam>]’
> /usr/include/c++/4.6/bits/stl_vector.h:840:9: instantiated from ‘void
> std::vector<_Tp, _Alloc>::push_back(std::vector<_Tp, _Alloc>::value_type&&)
> [with _Tp = {anonymous}::ModifierParam, _Alloc =
> std::allocator<{anonymous}::ModifierParam>, std::vector<_Tp,
> _Alloc>::value_type = {anonymous}::ModifierParam]’
> /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_t
> est_ccs_util.cpp:131:22: instantiated from here
> /usr/include/c++/4.6/bits/vector.tcc:319:4: error: use of deleted function
> ‘{anonymous}::ModifierParam& {anonymous}::ModifierParam::operator=(const
> {anonymous}::ModifierParam&)’
> /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_t
> est_ccs_util.cpp:94:11: error: ‘{anonymous}::ModifierParam&
> {anonymous}::ModifierParam::operator=(const {anonymous}::ModifierParam&)’ is
> implicitly deleted because the default definition would be ill-formed:
> /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfig/tests/compizconfig_t
> est_ccs_util.cpp:94:11: error: passing ‘const string {aka const
> std::basic_string<char>}’ as ‘this’ argument of ‘std::basic_string<_CharT,
> _Traits, _Alloc>& std::basic_string<_CharT, _Traits, _Alloc>::operator=(const
> std::basic_string<_CharT, _Traits, _Alloc>&) [with _CharT = char, _Traits =
> std::char_traits<char>, _Alloc = std::allocator<char>,
> std::basic_string<_CharT, _Traits, _Alloc> = std::basic_string<char>]’
> discards qualifiers [-fpermissive]
> In file included from /usr/include/c++/4.6/vector:61:0,
> from /usr/include/gtest/gtest.h:55,
> from /home/dan/bzr/compiz/tmp.577/compizconfig/libcompizconfi
> g/tests/compizconfig_test_ccs_util.cpp:22:
> /usr/include/c++/4.6/bits/stl_algobase.h: In static member function ‘static
> _BI2 std::__copy_move_backward<true, false,
> std::random_access_iterator_tag>::__copy_move_b(_BI1, _BI1, _BI2) [with _BI1 =
> {anonymous}::ModifierParam*, _BI2 = {anony...

[Daniel van Vugt (vanvugt)]

And I tried clang to get better error messages. But hit bug 1060804 instead.

[Sam Spilsbury (smspillaz)]

I cannot reproduce this compiler error but I've implemented copying semantics for the offending struct.

[Daniel van Vugt (vanvugt)]

Seems OK now.

review: Approve

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:3408
http://jenkins.qa.ubuntu.com/job/compiz-ci/160/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/160/console

review: Needs Fixing (continuous-integration)

[Sam Spilsbury (smspillaz)]

Note that tarmac will probably reject this due to the distro patch

[Unity Merger (unity-merger)]

The Jenkins job https://jenkins.qa.ubuntu.com/job/automerge-compiz-core/308/console reported an error when processing this lp:~compiz-team/compiz/compiz.fix_1058577 branch.
Not merging it.

[Sam Spilsbury (smspillaz)]

I think jenkins is using the new packaging repo with the removed conflicting patch, resubmitting.

[Sam Spilsbury (smspillaz)]

As above

review: Approve

[jenkins (martin-mrazik+qa)]

FAILED: Autolanding.
More details in the following jenkins job:
http://s-jenkins:8080/job/compiz-mbs-autolanding/1/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/compiz-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/1/console
    FAILURE: http://s-jenkins:8080/job/compiz-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/1/console

review: Needs Fixing (continuous-integration)

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:3408
http://jenkins.qa.ubuntu.com/job/compiz-ci/168/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/168/console

review: Needs Fixing (continuous-integration)

[Sam Spilsbury (smspillaz)]

review: Approve

[jenkins (martin-mrazik+qa)]

FAILED: Continuous integration, rev:3408
http://jenkins.qa.ubuntu.com/job/compiz-ci/170/
Executed test runs:
    FAILURE: http://jenkins.qa.ubuntu.com/job/compiz-ci/./build=pbuilder,distribution=quantal,flavor=amd64/170/console

review: Needs Fixing (continuous-integration)

[Daniel van Vugt (vanvugt)]

Isn't this the same revision I approved already in a different submission?

review: Approve

[Daniel van Vugt (vanvugt)]

P.S. Jenkins can't ever pass until the similar distro patch is removed from lp:ubuntu/compiz: primary_is_control.patch

[jenkins (martin-mrazik+qa)]

FAILED: Autolanding.
More details in the following jenkins job:
http://s-jenkins:8080/job/compiz-mbs-autolanding/2/
Executed test runs:
    FAILURE: http://s-jenkins:8080/job/compiz-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=amd64/2/console
    FAILURE: http://s-jenkins:8080/job/compiz-mbs-autolanding/./build=pbuilder,distribution=quantal,flavor=i386/2/console

review: Needs Fixing (continuous-integration)

[Sam Spilsbury (smspillaz)]

Right, although we've changed the packaging repo used for jenkins to lp:~compiz-team/compiz/ubuntu , although its still failing and the publisher is broken so we can't see why

[Daniel van Vugt (vanvugt)]

OK, I have hacked away the offending reference to primary_is_control.patch in lp:~compiz-team/compiz/ubuntu

Try again.

review: Approve

=== modified file 'compizconfig/libcompizconfig/src/bindings.c'
--- compizconfig/libcompizconfig/src/bindings.c	2012-09-25 06:12:24 +0000
+++ compizconfig/libcompizconfig/src/bindings.c	2012-10-10 12:52:21 +0000
@@ -38,14 +38,7 @@
 #include <X11/Xlib.h>
 
 #include <ccs.h>
-
-#define CompAltMask        (1 << 16)
-#define CompMetaMask       (1 << 17)
-#define CompSuperMask      (1 << 18)
-#define CompHyperMask      (1 << 19)
-#define CompModeSwitchMask (1 << 20)
-#define CompNumLockMask    (1 << 21)
-#define CompScrollLockMask (1 << 22)
+#include <ccs-modifier-list-inl.h>
 
 #define SCREEN_EDGE_LEFT	(1 << 0)
 #define SCREEN_EDGE_RIGHT	(1 << 1)
@@ -56,15 +49,10 @@
 #define SCREEN_EDGE_BOTTOMLEFT	(1 << 6)
 #define SCREEN_EDGE_BOTTOMRIGHT (1 << 7)
 
-struct _Modifier
-{
-    char *name;
-    int  modifier;
-}
-
-modifierList[] = {
+struct _Modifier modifierList[] = {
     { "<Shift>",      ShiftMask		 },
     { "<Control>",    ControlMask	 },
+    { "<Primary>",    ControlMask	 },
     { "<Mod1>",	      Mod1Mask		 },
     { "<Mod2>",	      Mod2Mask		 },
     { "<Mod3>",	      Mod3Mask		 },
@@ -77,8 +65,6 @@
     { "<ModeSwitch>", CompModeSwitchMask },
 };
 
-#define N_MODIFIERS (sizeof (modifierList) / sizeof (struct _Modifier))
-
 struct _Edge {
     char *name;
     char *modName;
@@ -98,9 +84,15 @@
 
 #define N_EDGES (sizeof (edgeList) / sizeof (edgeList[0]))
 
+unsigned int
+ccsInternalUtilNumModifiers ()
+{
+    return sizeof (modifierList) / sizeof (struct _Modifier);
+}
+
 static char *
-stringAppend (char *s,
-	      char *a)
+stringAppend (char       *s,
+	      const char *a)
 {
     char *r;
     int  len;
@@ -133,16 +125,48 @@
     return s;
 }
 
+void
+ccsAddKeybindingMaskToString (char         **bindingString,
+			      unsigned int matchBindingMask,
+			      unsigned int *addedBindingMask,
+			      unsigned int addBindingMask,
+			      const char   *addBindingString)
+{
+    if (addBindingMask & matchBindingMask &&
+	!(*addedBindingMask & addBindingMask))
+    {
+	*bindingString = stringAppend (*bindingString, addBindingString);
+	*addedBindingMask |= addBindingMask;
+    }
+}
+
+
+void
+ccsAddStringToKeybindingMask (unsigned int *bindingMask,
+			      const char   *bindingString,
+			      unsigned int addBindingMask,
+			      const char   *addBindingString)
+{
+    if (strcasestr (bindingString, addBindingString))
+    {
+	*bindingMask |= addBindingMask;
+    }
+}
+
 char *
 ccsModifiersToString (unsigned int modMask)
 {
-    char *binding = NULL;
+    char         *binding = NULL;
+    unsigned int addedBindings = 0;
     int  i;
 
-    for (i = 0; i < N_MODIFIERS; i++)
+    for (i = 0; i < ccsInternalUtilNumModifiers (); i++)
     {
-	if (modMask & modifierList[i].modifier)
-	    binding = stringAppend (binding, modifierList[i].name);
+	ccsAddKeybindingMaskToString (&binding,
+				      modMask,
+				      &addedBindings,
+				      modifierList[i].modifier,
+				      modifierList[i].name);
     }
 
     return binding;
@@ -235,10 +259,12 @@
     unsigned int mods = 0;
     int		 i;
 
-    for (i = 0; i < N_MODIFIERS; i++)
+    for (i = 0; i < ccsInternalUtilNumModifiers (); i++)
     {
-	if (strcasestr (binding, modifierList[i].name))
-	    mods |= modifierList[i].modifier;
+	ccsAddStringToKeybindingMask (&mods,
+				      binding,
+				      modifierList[i].modifier,
+				      modifierList[i].name);
     }
 
     return mods;
 
=== added file 'compizconfig/libcompizconfig/src/ccs-modifier-list-inl.h'
--- compizconfig/libcompizconfig/src/ccs-modifier-list-inl.h	1970-01-01 00:00:00 +0000
+++ compizconfig/libcompizconfig/src/ccs-modifier-list-inl.h	2012-10-10 12:52:21 +0000
@@ -0,0 +1,50 @@
+/*
+ * Compiz configuration system library
+ *
+ * Copyright (C) 2007  Danny Baumann <[e-mail]>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#ifndef _CCS_MODIFIER_LIST_INL_H
+#define _CCS_MODIFIER_LIST_INL_H
+
+#include <ccs-defs.h>
+
+COMPIZCONFIG_BEGIN_DECLS
+
+#include <X11/X.h>
+#include <X11/Xlib.h>
+
+#define CompAltMask        (1 << 16)
+#define CompMetaMask       (1 << 17)
+#define CompSuperMask      (1 << 18)
+#define CompHyperMask      (1 << 19)
+#define CompModeSwitchMask (1 << 20)
+#define CompNumLockMask    (1 << 21)
+#define CompScrollLockMask (1 << 22)
+
+struct _Modifier
+{
+    char *name;
+    int  modifier;
+};
+
+extern struct _Modifier modifierList[];
+unsigned int ccsInternalUtilNumModifiers ();
+
+COMPIZCONFIG_END_DECLS
+
+#endif
 
=== modified file 'compizconfig/libcompizconfig/src/ccs-private.h'
--- compizconfig/libcompizconfig/src/ccs-private.h	2012-09-10 01:06:55 +0000
+++ compizconfig/libcompizconfig/src/ccs-private.h	2012-10-10 12:52:21 +0000
@@ -22,6 +22,10 @@
 #ifndef CCS_PRIVATE_H
 #define CSS_PRIVATE_H
 
+#include <ccs-defs.h>
+
+COMPIZCONFIG_BEGIN_DECLS
+
 #include <ccs.h>
 #include <ccs-backend.h>
 
@@ -158,4 +162,19 @@
 unsigned int ccsAddConfigWatch (CCSContext            *context,
 				FileWatchCallbackProc callback);
 
+void
+ccsAddKeybindingMaskToString (char         **bindingString,
+			      unsigned int matchBindingMask,
+			      unsigned int *addedBindingMask,
+			      unsigned int addBindingMask,
+			      const char   *addBindingString);
+
+void
+ccsAddStringToKeybindingMask (unsigned int *bindingMask,
+			      const char   *bindingString,
+			      unsigned int addBindingMask,
+			      const char   *addBindingString);
+
+COMPIZCONFIG_END_DECLS
+
 #endif
 
=== modified file 'compizconfig/libcompizconfig/tests/CMakeLists.txt'
--- compizconfig/libcompizconfig/tests/CMakeLists.txt	2012-09-25 08:39:18 +0000
+++ compizconfig/libcompizconfig/tests/CMakeLists.txt	2012-10-10 12:52:21 +0000
@@ -32,6 +32,9 @@
 add_executable (compizconfig_test_ccs_mock_backend_conformance
 		${CMAKE_CURRENT_SOURCE_DIR}/compizconfig_test_ccs_mock_backend_conformance.cpp)
 
+add_executable (compizconfig_test_ccs_util
+		${CMAKE_CURRENT_SOURCE_DIR}/compizconfig_test_ccs_util.cpp)
+
 add_executable (compizconfig_test_ccs_upgrade_internal
     ${CMAKE_CURRENT_SOURCE_DIR}/compizconfig_test_ccs_settings_upgrade_internal.cpp)
 
@@ -136,6 +139,14 @@
 		       compizconfig_ccs_setting_value_matcher
 )
 
+target_link_libraries (compizconfig_test_ccs_util
+		       ${GTEST_BOTH_LIBRARIES}
+		       ${GMOCK_LIBRARY}
+		       ${GMOCK_MAIN_LIBRARY}
+		       ${CMAKE_THREAD_LIBS_INIT}
+                       compizconfig
+)
+
 compiz_discover_tests (compizconfig_test_ccs_object COVERAGE compizconfig)
 compiz_discover_tests (compizconfig_test_ccs_context COVERAGE compizconfig_ccs_context_mock)
 compiz_discover_tests (compizconfig_test_ccs_plugin COVERAGE compizconfig_ccs_plugin_mock)
@@ -143,3 +154,4 @@
 compiz_discover_tests (compizconfig_test_ccs_mock_backend_conformance COVERAGE compizconfig_ccs_backend_mock)
 compiz_discover_tests (compizconfig_test_ccs_text_file COVERAGE ccs_text_file_interface compizconfig_ccs_text_file_mock)
 compiz_discover_tests (compizconfig_test_ccs_upgrade_internal COVERAGE ccs_settings_upgrade_internal)
+compiz_discover_tests (compizconfig_test_ccs_util COVERAGE compizconfig)
 
=== added file 'compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp'
--- compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp	1970-01-01 00:00:00 +0000
+++ compizconfig/libcompizconfig/tests/compizconfig_test_ccs_util.cpp	2012-10-10 12:52:21 +0000
@@ -0,0 +1,238 @@
+/*
+ * Compiz configuration system library
+ *
+ * Copyright (C) 2007  Dennis Kasprzyk <[e-mail]>
+ * Copyright (C) 2007  Danny Baumann <[e-mail]>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ */
+
+#include <gtest/gtest.h>
+#include <gtest_shared_characterwrapper.h>
+
+#include <ccs.h>
+#include <ccs-private.h>
+#include <ccs-modifier-list-inl.h>
+
+using ::testing::WithParamInterface;
+
+namespace
+{
+    const std::string firstBindingString = "<First>";
+    const std::string secondBindingString = "<Second>";
+
+    const unsigned int firstBindingMask = (1 << 0);
+    const unsigned int secondBindingMask = (1 << 1);
+}
+
+TEST (CCSUtilTest, TestAddKeybindingMaskToStringInitial)
+{
+    char         *bindingStringChar = NULL;
+    unsigned int addedBindingMask = 0;
+
+    ccsAddKeybindingMaskToString (&bindingStringChar,
+				  firstBindingMask,
+				  &addedBindingMask,
+				  firstBindingMask,
+				  firstBindingString.c_str ());
+
+    CharacterWrapper bindingString (bindingStringChar);
+
+    EXPECT_EQ (firstBindingString, bindingStringChar);
+    EXPECT_EQ (addedBindingMask, firstBindingMask);
+}
+
+TEST (CCSUtilTest, TestAddKeybindingMaskToStringNoDuplicates)
+{
+    char         *bindingStringChar = NULL;
+    unsigned int addedBindingMask = 0;
+
+    ccsAddKeybindingMaskToString (&bindingStringChar,
+				  firstBindingMask,
+				  &addedBindingMask,
+				  firstBindingMask,
+				  firstBindingString.c_str ());
+
+    ccsAddKeybindingMaskToString (&bindingStringChar,
+				  firstBindingMask,
+				  &addedBindingMask,
+				  firstBindingMask,
+				  secondBindingString.c_str ());
+
+    CharacterWrapper bindingString (bindingStringChar);
+
+    EXPECT_EQ (firstBindingString, bindingStringChar);
+    EXPECT_EQ (addedBindingMask, firstBindingMask);
+}
+
+TEST (CCSUtilTest, TestAddStringToKeybindingMask)
+{
+    unsigned int bindingMask = 0;
+
+    ccsAddStringToKeybindingMask (&bindingMask,
+				  firstBindingString.c_str (),
+				  firstBindingMask,
+				  firstBindingString.c_str ());
+
+    EXPECT_EQ (bindingMask, firstBindingMask);
+}
+
+namespace
+{
+    class ModifierParam
+    {
+	public:
+
+	    ModifierParam (const char   *modifierString,
+			   unsigned int modifierMask,
+			   bool         match) :
+		mModifierString (modifierString),
+		mModifierMask (modifierMask),
+		mMatch (match)
+	    {
+	    }
+
+	    ModifierParam (const ModifierParam &param) :
+		mModifierString (param.mModifierString),
+		mModifierMask (param.mModifierMask),
+		mMatch (param.mMatch)
+	    {
+	    }
+
+	    friend void swap (ModifierParam &lhs, ModifierParam &rhs)
+	    {
+		using std::swap;
+
+		swap (lhs.mMatch, rhs.mMatch);
+		swap (lhs.mModifierMask, rhs.mModifierMask);
+		swap (lhs.mModifierString, rhs.mModifierString);
+	    }
+
+	    ModifierParam &
+	    operator= (const ModifierParam &other)
+	    {
+		ModifierParam to (other);
+		swap (*this, to);
+		return *this;
+	    }
+
+	    std::string       mModifierString;
+	    unsigned int      mModifierMask;
+	    bool              mMatch;
+    };
+
+    ::testing::internal::ParamGenerator<ModifierParam>
+    GenerateModifierParams ()
+    {
+	std::vector <ModifierParam> params;
+	params.reserve (ccsInternalUtilNumModifiers () *
+			ccsInternalUtilNumModifiers ());
+
+	for (unsigned int i = 0; i < ccsInternalUtilNumModifiers (); ++i)
+	{
+	    if (modifierList[i].name == std::string ("<Primary>"))
+		continue;
+
+	    for (unsigned int j = 0; j < ccsInternalUtilNumModifiers (); ++j)
+	    {
+		const bool modifierMatch = modifierList[i].modifier ==
+					   modifierList[j].modifier;
+
+		params.push_back (ModifierParam (modifierList[i].name,
+						 modifierList[j].modifier,
+						 modifierMatch));
+	    }
+	}
+
+	return ::testing::ValuesIn (params);
+    }
+
+    bool
+    CheckModifierListSanity ()
+    {
+	return (modifierList[0].modifier !=
+		modifierList[1].modifier) &&
+		(std::string (modifierList[0].name) !=
+		 std::string (modifierList[1].name));
+    }
+
+    const char *modifierSanityMsg = "This test requires the name and modifier " \
+				    "value in modifierList[0] and " \
+				    "modifierList[1] to be different " \
+				    "to work correctly";
+}
+
+class CCSUtilModifiersTest :
+    public ::testing::Test,
+    public WithParamInterface <ModifierParam>
+{
+};
+
+TEST_P (CCSUtilModifiersTest, TestModifiersToString)
+{
+    CharacterWrapper  modifierString (ccsModifiersToString (GetParam ().mModifierMask));
+    char              *modifierStringChar = modifierString;
+    /* Force "<Primary>" to test as "<Control>" as "<Primary>"
+     * should never be reachable */
+    const std::string expectedModifierString (GetParam ().mModifierString !=
+						  std::string ("<Primary>") ?
+					      GetParam ().mModifierString :
+					      "<Control>");
+
+    if (GetParam ().mMatch)
+	EXPECT_EQ (expectedModifierString, modifierStringChar);
+    else
+	EXPECT_NE (expectedModifierString, modifierStringChar);
+}
+
+TEST_P (CCSUtilModifiersTest, TestStringToModifiers)
+{
+    unsigned int modifierMask (ccsStringToModifiers (GetParam ().mModifierString.c_str ()));
+
+    if (GetParam ().mMatch)
+	EXPECT_EQ (GetParam ().mModifierMask, modifierMask);
+    else
+	EXPECT_NE (GetParam ().mModifierMask, modifierMask);
+}
+
+INSTANTIATE_TEST_CASE_P (CCSRealModifiers, CCSUtilModifiersTest,
+			 GenerateModifierParams ());
+
+TEST (CCSUtilModifierTest, TestMultiModifierToString)
+{
+    ASSERT_TRUE (CheckModifierListSanity ()) << modifierSanityMsg;
+
+    const unsigned int modifierMask           = modifierList[0].modifier |
+						modifierList[1].modifier;
+    const std::string  expectedModifierString = std::string (modifierList[0].name) +
+						std::string (modifierList[1].name);
+    CharacterWrapper   modifierString (ccsModifiersToString (modifierMask));
+    const char         *modifierStringChar = modifierString;
+
+    EXPECT_EQ (expectedModifierString, modifierStringChar);
+}
+
+TEST (CCSUtilModifierTest, TestMultiStringToModifier)
+{
+    ASSERT_TRUE (CheckModifierListSanity ()) << modifierSanityMsg;
+
+    const unsigned int expectedModifierMask = modifierList[0].modifier |
+					      modifierList[1].modifier;
+    const std::string  modifierString = std::string (modifierList[0].name) +
+					std::string (modifierList[1].name);
+    unsigned int       modifierMask = (ccsStringToModifiers (modifierString.c_str ()));
+
+    EXPECT_EQ (expectedModifierMask, modifierMask);
+}