Hi Andrew,

On 03/21/2016 05:10 PM, Andrew Zaborowski wrote:
> A do-it-all single method to add a signal filter, an alternative would
> be to add a bunch of functions for specific use cases.
> ---
>   ell/dbus.c | 123 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
>   ell/dbus.h |   7 ++++
>   2 files changed, 130 insertions(+)
>

Applied.  One more thought below:

> +LIB_EXPORT unsigned int l_dbus_add_signal_watch(struct l_dbus *dbus,
> +						const char *sender,
> +						const char *path,
> +						const char *interface,
> +						const char *member, ...)
> +{
> +	struct _dbus_filter_condition *rule;
> +	int rule_len;
> +	va_list args;
> +	const char *value;
> +	l_dbus_message_func_t signal_func;
> +	enum l_dbus_match_type type;
> +	void *user_data;
> +	unsigned int id;
> +
> +	va_start(args, member);
> +
> +	rule_len = 0;
> +	while ((type = va_arg(args, enum l_dbus_match_type)) !=
> +			L_DBUS_MATCH_NONE)
> +		rule_len++;
> +
> +	va_end(args);
> +
> +	rule = l_new(struct _dbus_filter_condition, rule_len + 5);
> +
> +	rule_len = 0;
> +
> +	rule[rule_len].type = L_DBUS_MATCH_TYPE;
> +	rule[rule_len++].value = "signal";
> +
> +	if (sender) {
> +		rule[rule_len].type = L_DBUS_MATCH_SENDER;
> +		rule[rule_len++].value = sender;
> +	}

Does it make sense to make glob-match values part of the tree?

For example, I can install watch 1 with:
type='signal', path='/'

and watch 2 with:
type='signal', sender='org.foo', path='/'

Right now it seems we will generate 2 AddMatch cases for the above, even 
though watch 2 is already covered by watch 1.

> +
> +	if (path) {
> +		rule[rule_len].type = L_DBUS_MATCH_PATH;
> +		rule[rule_len++].value = path;
> +	}
> +
> +	if (interface) {
> +		rule[rule_len].type = L_DBUS_MATCH_INTERFACE;
> +		rule[rule_len++].value = interface;
> +	}
> +
> +	if (member) {
> +		rule[rule_len].type = L_DBUS_MATCH_MEMBER;
> +		rule[rule_len++].value = member;
> +	}
> +
> +	va_start(args, member);
> +
> +	while (true) {
> +		type = va_arg(args, enum l_dbus_match_type);
> +		if (type == L_DBUS_MATCH_NONE)
> +			break;
> +
> +		value = va_arg(args, const char *);
> +
> +		rule[rule_len].type = type;
> +		rule[rule_len++].value = value;
> +	}
> +
> +	signal_func = va_arg(args, l_dbus_message_func_t);
> +	user_data = va_arg(args, void *);
> +
> +	va_end(args);
> +
> +	id = _dbus_filter_add_rule(dbus->filter, rule, rule_len,
> +					signal_func, user_data);
> +
> +	l_free(rule);
> +
> +	return id;
> +}

Regards,
-Denis