mirror of
https://gitlab.freedesktop.org/dbus/dbus.git
synced 2026-05-04 21:08:14 +02:00
Merge branch 'remove-nonexistent-match' into 'master'
bus: Don't return success from RemoveMatch if there was no such match-rule See merge request dbus/dbus!222
This commit is contained in:
Simon McVittie
commit
18b8883213
4 changed files with 205 additions and 51 deletions
21
bus/driver.c
21
bus/driver.c
|
|
@ -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;
|
||||
|
|
|
|||
|
|
@ -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
|
||||
|
|
|
|||
|
|
@ -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 */
|
||||
|
|
|
|||
|
|
@ -100,6 +100,7 @@ typedef struct {
|
|||
gchar *address;
|
||||
|
||||
DBusConnection *left_conn;
|
||||
gboolean left_conn_shouted_signal_filter;
|
||||
|
||||
DBusConnection *right_conn;
|
||||
GQueue held_messages;
|
||||
|
|
@ -107,6 +108,7 @@ typedef struct {
|
|||
gboolean right_conn_hold;
|
||||
gboolean wait_forever_called;
|
||||
guint activation_forking_counter;
|
||||
guint signal_counter;
|
||||
|
||||
gchar *tmp_runtime_dir;
|
||||
gchar *saved_runtime_dir;
|
||||
|
|
@ -158,6 +160,19 @@ hold_filter (DBusConnection *connection,
|
|||
return DBUS_HANDLER_RESULT_HANDLED;
|
||||
}
|
||||
|
||||
static DBusHandlerResult
|
||||
shouted_signal_filter (DBusConnection *connection,
|
||||
DBusMessage *message,
|
||||
void *user_data)
|
||||
{
|
||||
Fixture *f = user_data;
|
||||
|
||||
if (dbus_message_is_signal (message, "com.example", "Shouted"))
|
||||
f->signal_counter++;
|
||||
|
||||
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
|
||||
}
|
||||
|
||||
typedef struct {
|
||||
const char *bug_ref;
|
||||
guint min_messages;
|
||||
|
|
@ -236,6 +251,31 @@ add_hold_filter (Fixture *f)
|
|||
f->right_conn_hold = TRUE;
|
||||
}
|
||||
|
||||
static void
|
||||
add_shouted_signal_filter (Fixture *f)
|
||||
{
|
||||
if (!dbus_connection_add_filter (f->left_conn, shouted_signal_filter, f, NULL))
|
||||
g_error ("OOM");
|
||||
|
||||
f->left_conn_shouted_signal_filter = TRUE;
|
||||
}
|
||||
|
||||
static void
|
||||
right_conn_emit_shouted (Fixture *f)
|
||||
{
|
||||
DBusMessage *m;
|
||||
|
||||
m = dbus_message_new_signal ("/", "com.example", "Shouted");
|
||||
|
||||
if (m == NULL)
|
||||
g_error ("OOM");
|
||||
|
||||
if (!dbus_connection_send (f->right_conn, m, NULL))
|
||||
g_error ("OOM");
|
||||
|
||||
dbus_clear_message (&m);
|
||||
}
|
||||
|
||||
static void
|
||||
pc_count (DBusPendingCall *pc,
|
||||
void *data)
|
||||
|
|
@ -257,30 +297,11 @@ pc_enqueue (DBusPendingCall *pc,
|
|||
}
|
||||
|
||||
static void
|
||||
test_echo (Fixture *f,
|
||||
gconstpointer context)
|
||||
echo_left_to_right (Fixture *f,
|
||||
guint count)
|
||||
{
|
||||
const Config *config = context;
|
||||
guint count = 2000;
|
||||
guint sent;
|
||||
guint received = 0;
|
||||
double elapsed;
|
||||
|
||||
if (f->skip)
|
||||
return;
|
||||
|
||||
if (config != NULL && config->bug_ref != NULL)
|
||||
g_test_bug (config->bug_ref);
|
||||
|
||||
if (g_test_perf ())
|
||||
count = 100000;
|
||||
|
||||
if (config != NULL)
|
||||
count = MAX (config->min_messages, count);
|
||||
|
||||
add_echo_filter (f);
|
||||
|
||||
g_test_timer_start ();
|
||||
|
||||
for (sent = 0; sent < count; sent++)
|
||||
{
|
||||
|
|
@ -309,6 +330,33 @@ test_echo (Fixture *f,
|
|||
|
||||
while (received < count)
|
||||
test_main_context_iterate (f->ctx, TRUE);
|
||||
}
|
||||
|
||||
static void
|
||||
test_echo (Fixture *f,
|
||||
gconstpointer context)
|
||||
{
|
||||
const Config *config = context;
|
||||
guint count = 2000;
|
||||
double elapsed;
|
||||
|
||||
if (f->skip)
|
||||
return;
|
||||
|
||||
if (config != NULL && config->bug_ref != NULL)
|
||||
g_test_bug (config->bug_ref);
|
||||
|
||||
if (g_test_perf ())
|
||||
count = 100000;
|
||||
|
||||
if (config != NULL)
|
||||
count = MAX (config->min_messages, count);
|
||||
|
||||
add_echo_filter (f);
|
||||
|
||||
g_test_timer_start ();
|
||||
|
||||
echo_left_to_right (f, count);
|
||||
|
||||
elapsed = g_test_timer_elapsed ();
|
||||
|
||||
|
|
@ -2535,6 +2583,12 @@ teardown (Fixture *f,
|
|||
|
||||
if (f->left_conn != NULL)
|
||||
{
|
||||
if (f->left_conn_shouted_signal_filter)
|
||||
{
|
||||
dbus_connection_remove_filter (f->left_conn, shouted_signal_filter, f);
|
||||
f->left_conn_shouted_signal_filter = FALSE;
|
||||
}
|
||||
|
||||
test_connection_shutdown (f->ctx, f->left_conn);
|
||||
dbus_connection_close (f->left_conn);
|
||||
}
|
||||
|
|
@ -2674,6 +2728,71 @@ static Config send_destination_prefix_config = {
|
|||
TEST_USER_ME, SPECIFY_ADDRESS
|
||||
};
|
||||
|
||||
static void
|
||||
test_match_remove_fails (Fixture *f,
|
||||
gconstpointer context G_GNUC_UNUSED)
|
||||
{
|
||||
const char *match_rule = "type='signal'";
|
||||
|
||||
if (f->skip)
|
||||
return;
|
||||
|
||||
/* Unlike in test_match_remove_succeeds(), we never added this */
|
||||
dbus_bus_remove_match (f->left_conn, match_rule, &f->e);
|
||||
g_assert_cmpstr (f->e.name, ==, DBUS_ERROR_MATCH_RULE_NOT_FOUND);
|
||||
}
|
||||
|
||||
static void
|
||||
test_match_remove_succeeds (Fixture *f,
|
||||
gconstpointer context G_GNUC_UNUSED)
|
||||
{
|
||||
const char *match_rule = "type='signal'";
|
||||
|
||||
if (f->skip)
|
||||
return;
|
||||
|
||||
add_shouted_signal_filter (f);
|
||||
|
||||
/* We use this to make sure that a method call from the "left" connection
|
||||
* will get a reply from the "right", to sync up */
|
||||
add_echo_filter (f);
|
||||
|
||||
/* Emit a signal from the "right" connection, and assert that the "left"
|
||||
* does not receive it yet */
|
||||
f->signal_counter = 0;
|
||||
right_conn_emit_shouted (f);
|
||||
/* Because messages are totally-ordered, if the "left" connection was
|
||||
* going to receive the signal, it would receive it before it got
|
||||
* the reply from this async call to the "right" connection */
|
||||
echo_left_to_right (f, 1);
|
||||
g_assert_cmpuint (f->signal_counter, ==, 0);
|
||||
|
||||
dbus_bus_add_match (f->left_conn, match_rule, &f->e);
|
||||
test_assert_no_error (&f->e);
|
||||
|
||||
f->signal_counter = 0;
|
||||
|
||||
/* Emit a signal from the "right" connection, and assert that the "left"
|
||||
* receives it */
|
||||
right_conn_emit_shouted (f);
|
||||
|
||||
while (f->signal_counter < 1)
|
||||
test_main_context_iterate (f->ctx, TRUE);
|
||||
|
||||
dbus_bus_remove_match (f->left_conn, match_rule, &f->e);
|
||||
test_assert_no_error (&f->e);
|
||||
|
||||
/* Emit a signal from the "right" connection, and assert that the "left"
|
||||
* does not receive it this time */
|
||||
f->signal_counter = 0;
|
||||
right_conn_emit_shouted (f);
|
||||
/* Because messages are totally-ordered, if the "left" connection was
|
||||
* going to receive the signal, it would receive it before it got
|
||||
* the reply from this async call to the "right" connection */
|
||||
echo_left_to_right (f, 1);
|
||||
g_assert_cmpuint (f->signal_counter, ==, 0);
|
||||
}
|
||||
|
||||
int
|
||||
main (int argc,
|
||||
char **argv)
|
||||
|
|
@ -2708,6 +2827,10 @@ main (int argc,
|
|||
g_test_add ("/limits/max-names-per-connection", Fixture,
|
||||
&max_names_per_connection_config,
|
||||
setup, test_max_names_per_connection, teardown);
|
||||
g_test_add ("/match/remove/fails", Fixture, NULL,
|
||||
setup, test_match_remove_fails, teardown);
|
||||
g_test_add ("/match/remove/succeeds", Fixture, NULL,
|
||||
setup, test_match_remove_succeeds, teardown);
|
||||
g_test_add ("/peer/ping", Fixture, NULL, setup, test_peer_ping, teardown);
|
||||
g_test_add ("/peer/get-machine-id", Fixture, NULL,
|
||||
setup, test_peer_get_machine_id, teardown);
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue