Merge lp:~vanvugt/compiz/fix-1080555 into lp:compiz/0.9.9

Proposed by Daniel van Vugt
Status: Merged
Approved by: Timo Jyrinki
Approved revision: 3475
Merged at revision: 3476
Proposed branch: lp:~vanvugt/compiz/fix-1080555
Merge into: lp:compiz/0.9.9
Diff against target: 40 lines (+6/-2)
3 files modified
compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting_factory.c (+3/-0)
compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_wrapper_factory.c (+2/-1)
compizconfig/libcompizconfig/src/main.c (+1/-1)
To merge this branch: bzr merge lp:~vanvugt/compiz/fix-1080555
Reviewer Review Type Date Requested Status
PS Jenkins bot (community) continuous-integration Approve
Compiz Maintainers Pending
Review via email: mp+134833@code.launchpad.net

Commit message

Fix segfaults in test cases (LP: #1080555)

Description of the change

Some recent change to the compizconfig code has caused these new segfaults (LP: #1080555) which are masking the OTHER_FAULT problem (LP: #1070817).

To post a comment you must log in.
lp:~vanvugt/compiz/fix-1080555 updated
3475. By Daniel van Vugt

Remove reformatting not related to the bug fix.

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote :
Download full text (3.6 KiB)

The null check here is harmless but it is masking the real issue at hand.
The Python tests should always be using the ini back end, so that needs to
be looked into.

review approve
On 19/11/2012 2:12 PM, "Daniel van Vugt" <email address hidden>
wrote:

> Daniel van Vugt has proposed merging lp:~vanvugt/compiz/fix-1080555 into
> lp:compiz.
>
> Commit message:
> Fix segfaults in test cases (LP: #1080555)
>
>
> Requested reviews:
> Compiz Maintainers (compiz-team)
> Related bugs:
> Bug #1080555 in Compiz: "Tests 1-4 fail with SEGFAULT:
> CompizConfigPython.test_* (SEGFAULT)..."
> https://bugs.launchpad.net/compiz/+bug/1080555
>
> For more details, see:
> https://code.launchpad.net/~vanvugt/compiz/fix-1080555/+merge/134833
>
> Some recent change to the compizconfig code has caused these new segfaults
> (LP: #1080555) which are masking the OTHER_FAULT problem (LP: #1070817).
> --
> https://code.launchpad.net/~vanvugt/compiz/fix-1080555/+merge/134833
> Your team Compiz Maintainers is requested to review the proposed merge of
> lp:~vanvugt/compiz/fix-1080555 into lp:compiz.
>
> === modified file
> 'compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting_factory.c'
> ---
> compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting_factory.c
> 2012-10-08 13:50:56 +0000
> +++
> compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting_factory.c
> 2012-11-19 06:11:19 +0000
> @@ -179,6 +179,9 @@
> SpecialOptionType specialType = (SpecialOptionType)
> GPOINTER_TO_INT (g_hash_table_lookup (settingsSpecialTypesHashTable,
> settingName));
> const gchar *integratedName = g_hash_table_lookup
> (settingsSettingNameGNOMENameHashTable, settingName);
>
> + if (wrapper == NULL)
> + return NULL;
> +
> return createNewGSettingsIntegratedSetting (wrapper,
> integratedName,
> pluginName,
>
> === modified file
> 'compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_wrapper_factory.c'
> ---
> compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_wrapper_factory.c
> 2012-11-14 06:06:55 +0000
> +++
> compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_wrapper_factory.c
> 2012-11-19 06:11:19 +0000
> @@ -66,11 +66,13 @@
> {
> CCSGNOMEIntegrationGSettingsWrapperFactoryPrivate *priv =
> GET_PRIVATE
> (CCSGNOMEIntegrationGSettingsWrapperFactoryPrivate, factory);
> - CCSGSettingsWrapper *wrapper =
> ccsGSettingsWrapperFactoryNewGSettingsWrapper (priv->wrapperFactory,
> -
> schemaName,
> -
> factory->object.object_allocation);
> + CCSGSettingsWrapper *wrapper =
> + ccsGSettingsWrapperFactoryNewGSettingsWrapper
> (priv->wrapperFactory,
> + schemaName,
> +
> factory->object.object_allocation);
>
> - connectWrapperToChangedSignal (wrapper, priv);
> + if (wrapper != NULL)
> + connectWrapperToChangedSignal (wrapper, priv)...

Read more...

Revision history for this message
Daniel van Vugt (vanvugt) wrote :

Sam, you know you're back in compiz-team so should be able to approve?

Revision history for this message
PS Jenkins bot (ps-jenkins) wrote :
review: Approve (continuous-integration)
Revision history for this message
Sam Spilsbury (smspillaz) wrote :

Sure, I just only had email access back then :)

I trust that someone is looking into the fact that those tests are not
using the ini backend? that's the real issue at hand.

On Mon, Nov 19, 2012 at 4:21 PM, Daniel van Vugt
<email address hidden> wrote:
> Sam, you know you're back in compiz-team so should be able to approve?
> --
> https://code.launchpad.net/~vanvugt/compiz/fix-1080555/+merge/134833
> Your team Compiz Maintainers is requested to review the proposed merge of lp:~vanvugt/compiz/fix-1080555 into lp:compiz.

--
Sam Spilsbury

Preview Diff

[H/L] Next/Prev Comment, [J/K] Next/Prev File, [N/P] Next/Prev Hunk
=== modified file 'compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting_factory.c'
--- compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting_factory.c 2012-10-08 13:50:56 +0000
+++ compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting_factory.c 2012-11-19 06:23:21 +0000
@@ -179,6 +179,9 @@
179 SpecialOptionType specialType = (SpecialOptionType) GPOINTER_TO_INT (g_hash_table_lookup (settingsSpecialTypesHashTable, settingName));179 SpecialOptionType specialType = (SpecialOptionType) GPOINTER_TO_INT (g_hash_table_lookup (settingsSpecialTypesHashTable, settingName));
180 const gchar *integratedName = g_hash_table_lookup (settingsSettingNameGNOMENameHashTable, settingName);180 const gchar *integratedName = g_hash_table_lookup (settingsSettingNameGNOMENameHashTable, settingName);
181181
182 if (wrapper == NULL)
183 return NULL;
184
182 return createNewGSettingsIntegratedSetting (wrapper,185 return createNewGSettingsIntegratedSetting (wrapper,
183 integratedName,186 integratedName,
184 pluginName,187 pluginName,
185188
=== modified file 'compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_wrapper_factory.c'
--- compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_wrapper_factory.c 2012-11-14 06:06:55 +0000
+++ compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_wrapper_factory.c 2012-11-19 06:23:21 +0000
@@ -70,7 +70,8 @@
70 schemaName,70 schemaName,
71 factory->object.object_allocation);71 factory->object.object_allocation);
7272
73 connectWrapperToChangedSignal (wrapper, priv);73 if (wrapper != NULL)
74 connectWrapperToChangedSignal (wrapper, priv);
7475
75 return wrapper;76 return wrapper;
76}77}
7778
=== modified file 'compizconfig/libcompizconfig/src/main.c'
--- compizconfig/libcompizconfig/src/main.c 2012-10-26 11:52:31 +0000
+++ compizconfig/libcompizconfig/src/main.c 2012-11-19 06:23:21 +0000
@@ -5727,7 +5727,7 @@
57275727
5728 while (iter)5728 while (iter)
5729 {5729 {
5730 if ((*pred) (iter->data, data))5730 if (iter->data != NULL && pred (iter->data, data))
5731 returnList = ccsIntegratedSettingListAppend (returnList, iter->data);5731 returnList = ccsIntegratedSettingListAppend (returnList, iter->data);
57325732
5733 iter = iter->next;5733 iter = iter->next;

Subscribers

People subscribed via source and target branches