bus: Separate RemoveMatch into prepare and commit stages

This means we don't send a spurious successful reply if a caller removes
a match rule that they never added.

Signed-off-by: Simon McVittie <smcv@collabora.com>
This commit is contained in:
Simon McVittie 2021-11-22 16:01:58 +00:00
parent 32278953a1
commit 81a5731bcb
3 changed files with 61 additions and 30 deletions

View file

@ -1409,6 +1409,7 @@ bus_driver_handle_remove_match (DBusConnection *connection,
const char *text;
DBusString str;
BusMatchmaker *matchmaker;
DBusList *link;
_DBUS_ASSERT_ERROR_IS_CLEAR (error);
@ -1429,17 +1430,21 @@ bus_driver_handle_remove_match (DBusConnection *connection,
if (rule == NULL)
goto failed;
/* Send the ack before we remove the rule, since the ack is undone
* on transaction cancel, but rule removal isn't.
*/
matchmaker = bus_connection_get_matchmaker (connection);
/* Check whether the rule exists and prepare to remove it, but don't
* actually do it yet. */
link = bus_matchmaker_prepare_remove_rule_by_value (matchmaker, rule, error);
if (link == NULL)
goto failed;
/* We do this before actually removing the rule, because removing the
* rule cannot be undone if we run out of memory here. */
if (!bus_driver_send_ack_reply (connection, transaction, message, error))
goto failed;
matchmaker = bus_connection_get_matchmaker (connection);
if (!bus_matchmaker_remove_rule_by_value (matchmaker, rule, error))
goto failed;
/* The rule exists, so now we can do things we can't undo. */
bus_matchmaker_commit_remove_rule_by_value (matchmaker, rule, link);
bus_match_rule_unref (rule);
return TRUE;

View file

@ -1608,21 +1608,32 @@ bus_matchmaker_remove_rule (BusMatchmaker *matchmaker,
bus_match_rule_unref (rule);
}
/* Remove a single rule which is equal to the given rule by value */
dbus_bool_t
bus_matchmaker_remove_rule_by_value (BusMatchmaker *matchmaker,
BusMatchRule *value,
DBusError *error)
/*
* Prepare to remove the the most-recently-added rule which is equal to
* the given rule by value, but do not actually do it yet.
*
* Return a linked-list link which must be treated as opaque by the caller:
* the only valid thing to do with it is to pass it to
* bus_matchmaker_commit_remove_rule_by_value().
*
* The returned linked-list link becomes invalid when control returns to
* the main loop. If the caller decides not to remove the rule after all,
* there is currently no need to cancel explicitly.
*/
DBusList *
bus_matchmaker_prepare_remove_rule_by_value (BusMatchmaker *matchmaker,
BusMatchRule *value,
DBusError *error)
{
DBusList **rules;
DBusList *link = NULL;
_dbus_verbose ("Removing rule by value with message_type %d, interface %s\n",
_dbus_verbose ("Finding rule by value with message_type %d, interface %s\n",
value->message_type,
value->interface != NULL ? value->interface : "<null>");
rules = bus_matchmaker_get_rules (matchmaker, value->message_type,
value->interface, FALSE);
value->interface, FALSE);
if (rules != NULL)
{
@ -1639,26 +1650,37 @@ bus_matchmaker_remove_rule_by_value (BusMatchmaker *matchmaker,
prev = _dbus_list_get_prev_link (rules, link);
if (match_rule_equal (rule, value))
{
bus_matchmaker_remove_rule_link (rules, link);
break;
}
return link;
link = prev;
}
}
if (link == NULL)
{
dbus_set_error (error, DBUS_ERROR_MATCH_RULE_NOT_FOUND,
"The given match rule wasn't found and can't be removed");
return FALSE;
}
dbus_set_error (error, DBUS_ERROR_MATCH_RULE_NOT_FOUND,
"The given match rule wasn't found and can't be removed");
return NULL;
}
/*
* Commit a previous call to bus_matchmaker_prepare_remove_rule_by_value(),
* which must have been done during the same main-loop iteration.
*/
void
bus_matchmaker_commit_remove_rule_by_value (BusMatchmaker *matchmaker,
BusMatchRule *value,
DBusList *link)
{
DBusList **rules;
_dbus_assert (match_rule_equal (link->data, value));
rules = bus_matchmaker_get_rules (matchmaker, value->message_type,
value->interface, FALSE);
/* Should only be called if a rule matching value was successfully
* added, which means rules must contain at least link */
_dbus_assert (rules != NULL);
bus_matchmaker_remove_rule_link (rules, link);
bus_matchmaker_gc_rules (matchmaker, value->message_type, value->interface,
rules);
return TRUE;
}
static void

View file

@ -91,9 +91,6 @@ void bus_matchmaker_unref (BusMatchmaker *matchmaker);
dbus_bool_t bus_matchmaker_add_rule (BusMatchmaker *matchmaker,
BusMatchRule *rule);
dbus_bool_t bus_matchmaker_remove_rule_by_value (BusMatchmaker *matchmaker,
BusMatchRule *value,
DBusError *error);
void bus_matchmaker_remove_rule (BusMatchmaker *matchmaker,
BusMatchRule *rule);
void bus_matchmaker_disconnected (BusMatchmaker *matchmaker,
@ -105,4 +102,11 @@ dbus_bool_t bus_matchmaker_get_recipients (BusMatchmaker *matchmaker,
DBusMessage *message,
DBusList **recipients_p);
DBusList *bus_matchmaker_prepare_remove_rule_by_value (BusMatchmaker *matchmaker,
BusMatchRule *value,
DBusError *error);
void bus_matchmaker_commit_remove_rule_by_value (BusMatchmaker *matchmaker,
BusMatchRule *value,
DBusList *link);
#endif /* BUS_SIGNALS_H */