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
1=== modified file 'compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting_factory.c'
2--- compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting_factory.c 2012-10-08 13:50:56 +0000
3+++ compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_integrated_setting_factory.c 2012-11-19 06:23:21 +0000
4@@ -179,6 +179,9 @@
5 SpecialOptionType specialType = (SpecialOptionType) GPOINTER_TO_INT (g_hash_table_lookup (settingsSpecialTypesHashTable, settingName));
6 const gchar *integratedName = g_hash_table_lookup (settingsSettingNameGNOMENameHashTable, settingName);
7
8+ if (wrapper == NULL)
9+ return NULL;
10+
11 return createNewGSettingsIntegratedSetting (wrapper,
12 integratedName,
13 pluginName,
14
15=== modified file 'compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_wrapper_factory.c'
16--- compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_wrapper_factory.c 2012-11-14 06:06:55 +0000
17+++ compizconfig/integration/gnome/gsettings/src/ccs_gnome_integration_gsettings_wrapper_factory.c 2012-11-19 06:23:21 +0000
18@@ -70,7 +70,8 @@
19 schemaName,
20 factory->object.object_allocation);
21
22- connectWrapperToChangedSignal (wrapper, priv);
23+ if (wrapper != NULL)
24+ connectWrapperToChangedSignal (wrapper, priv);
25
26 return wrapper;
27 }
28
29=== modified file 'compizconfig/libcompizconfig/src/main.c'
30--- compizconfig/libcompizconfig/src/main.c 2012-10-26 11:52:31 +0000
31+++ compizconfig/libcompizconfig/src/main.c 2012-11-19 06:23:21 +0000
32@@ -5727,7 +5727,7 @@
33
34 while (iter)
35 {
36- if ((*pred) (iter->data, data))
37+ if (iter->data != NULL && pred (iter->data, data))
38 returnList = ccsIntegratedSettingListAppend (returnList, iter->data);
39
40 iter = iter->next;

Subscribers

People subscribed via source and target branches