libnm-core: fix up connection deserialize/copy/replace semantics

libnm-util's connection deserializing/copying/replacing functions have
odd semantics where sometimes they both modify a connection AND return
an error. libnm-core tried to improve things by guaranteeing that the
connection would not be modified if the new settings were invalid, but
this ended up breaking a bunch of places that needed to be able to
work with invalid connections. So re-fix the functions by reverting
back to the old semantics, but having return values that clearly
distinguish whether the connection was modified or not.

For comparison:

  - nm_connection_new_from_hash() / nm_simple_connection_new_from_dbus():

      - libnm-util: returns a valid connection or NULL.

      - OLD libnm-core: returned a valid connection or NULL.

      - NEW libnm-core: returns a valid connection or NULL.

  - nm_connection_duplicate() / nm_simple_connection_new_clone():

      - libnm-util: always succeeds, whether or not the connection is
        valid.

      - OLD libnm-core: returned a valid connection or NULL

      - NEW libnm-core: always succeeds, whether or not the connection
        is valid.

    - nm_connection_replace_settings_from_connection():

      - libnm-util: always replaces the settings, but returns FALSE if
        the connection is now invalid.

      - OLD libnm-core: either replaced the settings and returned TRUE
        (if the settings were valid), or else left the connection
        unchanged and returned FALSE (if not).

      - NEW libnm-core: always replaces the settings, and has no
        return value. (The modified connection is valid if and only if
        the replaced-from connection was valid; just like with the
        libnm-util version.)

    - nm_connection_replace_settings():

      - libnm-util: returns TRUE if the new settings are valid, or
        FALSE if either (a) the new settings could not be deserialized
        and the connection is unchanged, or (b) the new settings were
        deserialized, and the connection was updated, but is now not
        valid.

      - OLD libnm-core: either replaced the settings and returned TRUE
        (if the settings were valid), or else left the connection
        unchanged and returned FALSE (if not).

      - NEW libnm-core: returns TRUE if the connection was updated
        (whether or not it is valid), or FALSE if the new settings
        could not be deserialized and the connection is unchanged.

1 files changed, 22 insertions, 25 deletions

diff --git a/src/settings/nm-settings-connection.c b/src/settings/nm-settings-connection.c
index 9e9f56b28d..513e6efc13 100644
--- a/src/settings/nm-settings-connection.c
+++ b/src/settings/nm-settings-connection.c
@@ -463,37 +463,34 @@ nm_settings_connection_replace_settings (NMSettingsConnection *self,
 	 */
 	g_signal_handlers_block_by_func (self, G_CALLBACK (changed_cb), GUINT_TO_POINTER (TRUE));
 
-	if (nm_connection_replace_settings_from_connection (NM_CONNECTION (self),
-	                                                    new_connection,
-	                                                    error)) {
-		priv->nm_generated = FALSE;
+	nm_connection_replace_settings_from_connection (NM_CONNECTION (self), new_connection);
+	priv->nm_generated = FALSE;
 
-		/* Cache the just-updated system secrets in case something calls
-		 * nm_connection_clear_secrets() and clears them.
-		 */
-		update_system_secrets_cache (self);
-		success = TRUE;
+	/* Cache the just-updated system secrets in case something calls
+	 * nm_connection_clear_secrets() and clears them.
+	 */
+	update_system_secrets_cache (self);
+	success = TRUE;
 
-		/* Add agent and always-ask secrets back; they won't necessarily be
-		 * in the replacement connection data if it was eg reread from disk.
-		 */
-		if (priv->agent_secrets) {
-			hash = nm_connection_to_dbus (priv->agent_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
-			if (hash) {
-				(void) nm_connection_update_secrets (NM_CONNECTION (self), NULL, hash, NULL);
-				g_hash_table_destroy (hash);
-			}
+	/* Add agent and always-ask secrets back; they won't necessarily be
+	 * in the replacement connection data if it was eg reread from disk.
+	 */
+	if (priv->agent_secrets) {
+		hash = nm_connection_to_dbus (priv->agent_secrets, NM_CONNECTION_SERIALIZE_ONLY_SECRETS);
+		if (hash) {
+			(void) nm_connection_update_secrets (NM_CONNECTION (self), NULL, hash, NULL);
+			g_hash_table_destroy (hash);
 		}
+	}
 
-		nm_settings_connection_recheck_visibility (self);
+	nm_settings_connection_recheck_visibility (self);
 
-		/* Manually emit changed signal since we disconnected the handler, but
-		 * only update Unsaved if the caller wanted us to.
-		 */
-		changed_cb (self, GUINT_TO_POINTER (update_unsaved));
+	/* Manually emit changed signal since we disconnected the handler, but
+	 * only update Unsaved if the caller wanted us to.
+	 */
+	changed_cb (self, GUINT_TO_POINTER (update_unsaved));
 
-		g_signal_emit (self, signals[UPDATED_BY_USER], 0);
-	}
+	g_signal_emit (self, signals[UPDATED_BY_USER], 0);
 
 	g_signal_handlers_unblock_by_func (self, G_CALLBACK (changed_cb), GUINT_TO_POINTER (TRUE));