Code review comment for lp:~vanvugt/compiz/fix-1080555

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);
>
> return wrapper;
> }
>
> === 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:11:19 +0000
> @@ -5727,7 +5727,7 @@
>
> while (iter)
> {
> - if ((*pred) (iter->data, data))
> + if (iter->data != NULL && pred (iter->data, data))
> returnList = ccsIntegratedSettingListAppend (returnList,
> iter->data);
>
> iter = iter->next;
>
>
>