Re: [PATCH 1/3] Add SetRemoteProperties method for OOB COD setting

[Johan Hedberg <johan.hedberg@gmail.com>]

Hi Bartosz,

On Fri, Aug 12, 2011, Bartosz Szatkowski wrote:
> +struct oob_remote_parameters {
> +	bdaddr_t local;
> +	bdaddr_t peer;
> +	const char *address;
> +	uint32_t class;
> +};

Could you add a "gboolean device_found;" to this struct and then use it
as follows:

> +static DBusMessage *parse_class(DBusMessageIter *value,
> +			struct oob_remote_parameters *params, DBusMessage *msg)
> +{
> +	if (dbus_message_iter_get_arg_type(value) != DBUS_TYPE_UINT32)
> +		return btd_error_invalid_args(msg);
> +
> +	dbus_message_iter_get_basic(value, &params->class);

	params->device_found = TRUE;

> +	return NULL;
> +}
> +
> +static gboolean set_class(struct oob_remote_parameters *params)
> +{
> +	if (params->class == 0)
> +		return FALSE;

You can move the above if-statement here:

> +static void set_properties(struct btd_adapter *adapter,
> +					struct oob_remote_parameters *params)
> +{
> +	gboolean device_found = FALSE;
> +
> +	if (set_class(params))
> +		device_found = TRUE;

I.e. change this to:

	if (params->class != 0)
		write_remote_class(...);

I suppose you could keep the set_class function if you want, but since
all it does is make a single write_remote_class call you might as well
do the call directly as above.

> +	if (device_found)
> +		emit_device_found(adapter_get_path(adapter), params);
> +}

The above would become:

	if (params->device_found)
		emit_device_found(..., params);

With these changes the flow of logic becomes a bit easier to follow (to
me at least).

Johan