[Bug 27727] account storage plugins can contain partial accounts

[bugzilla-daemon at freedesktop.org]

https://bugs.freedesktop.org/show_bug.cgi?id=27727

--- Comment #36 from Xavier Claessens <xclaesse at gmail.com> ---
Comment on attachment 89226
  --> https://bugs.freedesktop.org/attachment.cgi?id=89226
19/26] mcp_account_storage_set_*: return whether anything  changed

Review of attachment 89226:
 --> (https://bugs.freedesktop.org/page.cgi?id=splinter.html&bug=27727&attachment=89226)
-----------------------------------------------------------------

::: src/mcd-account-manager-default.c
@@ +182,3 @@
>  
> +      if (sa == NULL)
> +        return MCP_ACCOUNT_STORAGE_SET_RESULT_UNCHANGED;

I would make it g_return_val_if_fail (sa != NULL,
MCP_ACCOUNT_STORAGE_SET_RESULT_FAILED); because we are no longer setting on all
backends. So when MC tells a plugin to set a value, the storage must know about
that account.

@@ +194,5 @@
> +  else
> +    {
> +      GVariant *old;
> +
> +      sa = ensure_stored_account (amd, account);

ensure_stored_account() should only be called from _create(), right? Same as
above, I would g_return_val_if_fail().

@@ +203,5 @@
> +          /* it might still be in untyped_parameters? */
> +          const gchar *escaped = g_hash_table_lookup (sa->untyped_parameters,
> +              parameter);
> +          gchar *new = mcp_account_manager_escape_variant_for_keyfile (
> +              am, val);

If we have an untyped param, now is a good time to move to to sa->parameters
and consider that to be a change.

@@ +243,5 @@
> +    {
> +      sa = lookup_stored_account (amd, account);
> +
> +      if (sa == NULL)
> +        return MCP_ACCOUNT_STORAGE_SET_RESULT_UNCHANGED;

same as above, g_return_val_if_fail()

@@ +255,2 @@
>  
> +      sa = ensure_stored_account (amd, account);

same as above: s/ensure/lookup/ and g_return_val_if_fail()

-- 
You are receiving this mail because:
You are the QA Contact for the bug.