Re: [PATCH v3 1/4] dbus: Add AddMatch and RemoveMatch support

[Denis Kenzior <denkenz@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2155 bytes --]

Hi Jukka,

On 02/18/2015 10:08 AM, Jukka Rissanen wrote:
> We do not check for errors as they are typically unrecoverable
> in this case and in most cases ignored anyway.
>
> This is the comment from libdbus API documentation for
> dbus_bus_add_match() about error checking:
>
> "If you pass NULL for the error, this function will not block;
> the match thus won't be added until you flush the connection,
> and if there's an error adding the match you won't find out
> about it. This is generally acceptable, since the possible
> errors (including a lack of resources in the bus, the connection
> having exceeded its quota of active match rules, or the match
> rule being unparseable) are generally unrecoverable."
> ---
>   ell/dbus.c | 26 ++++++++++++++++++++++++++
>   1 file changed, 26 insertions(+)
>

Mostly fine with this one, but please tweak the naming to be consistent 
with what we already have.

> diff --git a/ell/dbus.c b/ell/dbus.c
> index 4c4ba2d..c10e0ed 100644
> --- a/ell/dbus.c
> +++ b/ell/dbus.c
> @@ -1227,3 +1227,29 @@ LIB_EXPORT bool l_dbus_unregister_interface(struct l_dbus *dbus,
>
>   	return _dbus_object_tree_unregister(dbus->tree, path, interface);
>   }
> +
> +static void send_match(struct l_dbus *dbus, const char *rule,
> +						const char *method)

So dbus1_send_match

> +{
> +	struct l_dbus_message *message;
> +
> +	message = l_dbus_message_new_method_call(dbus,
> +						DBUS_SERVICE_DBUS,
> +						DBUS_PATH_DBUS,
> +						DBUS_INTERFACE_DBUS,
> +						method);
> +
> +	l_dbus_message_set_arguments(message, "s", rule);
> +
> +	send_message(dbus, false, message, NULL, NULL, NULL);
> +}
> +
> +static void _dbus_bus_add_match(struct l_dbus *dbus, const char *rule)

The '_' prefix is reserved to exported functions that are private. 
Since this function is static, do not use the '_' prefix.

dbus1_bus_add_match

> +{
> +	send_match(dbus, rule, "AddMatch");
> +}
> +
> +static void _dbus_bus_remove_match(struct l_dbus *dbus, const char *rule)
> +{

dbus1_remove_match

> +	send_match(dbus, rule, "RemoveMatch");
> +}
>

Regards,
-Denis