Re: [PATCHv1 2/5] service: Retrieve get_conversation() args

[Marcel Holtmann <marcel@holtmann.org>]

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

Hi Ronald,

> ---
>  src/service.c |   37 +++++++++++++++++++++++++++++++++++++
>  1 files changed, 37 insertions(+), 0 deletions(-)
> 
> diff --git a/src/service.c b/src/service.c
> index 24c89b6..18c11f4 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -690,11 +690,48 @@ static DBusMessage *get_messages(DBusConnection *conn,
>  	return reply;
>  }
> 
> +static gboolean get_conversation_get_args(DBusMessage *dbus_msg,
> +				const char **number, unsigned int *count)
> +{
> +	DBusMessageIter top_iter;
> +
> +	if (dbus_message_iter_init(dbus_msg, &top_iter) == FALSE)
> +		return FALSE;
> +
> +	if (dbus_message_iter_get_arg_type(&top_iter) != DBUS_TYPE_STRING)
> +		return FALSE;
> +
> +	dbus_message_iter_get_basic(&top_iter, number);
> +	if (*number[0] == '\0')
> +		return FALSE;
> +
> +	if (valid_number_format(*number) == FALSE)
> +		return FALSE;
> +
> +	if (!dbus_message_iter_next(&top_iter))
> +		return FALSE;
> +
> +	if (dbus_message_iter_get_arg_type(&top_iter) != DBUS_TYPE_UINT32)
> +		return FALSE;
> +
> +	dbus_message_iter_get_basic(&top_iter, count);
> +
> +	return TRUE;
> +}
> +
>  static DBusMessage *get_conversation(DBusConnection *conn,
>  					DBusMessage *dbus_msg, void *data)
>  {
>  	DBusMessage *reply;
>  	DBusMessageIter iter, array;
> +	const char *number;
> +	unsigned int count;
> +
> +	if (get_conversation_get_args(dbus_msg, &number, &count) == FALSE) {
> +		mms_debug("Invalid arguments");
> +
> +		return __mms_error_invalid_args(dbus_msg);
> +	}

in this case I don't really see that it is useful to split this into a
separate function. And the debug print is actually useless. Please
remove that one.

> 
>  	reply = dbus_message_new_method_return(dbus_msg);
>  	if (reply == NULL)

Regards

Marcel