Re: [PATCH 1/2] dbus: Add private _dbus_message_new_error

[Denis Kenzior <denkenz@gmail.com>]

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

Hi Andrew,

On 04/18/2016 09:31 AM, Andrew Zaborowski wrote:
> ---
>   ell/dbus-message.c | 47 +++++++++++++++++++++++++++++++----------------
>   ell/dbus-private.h |  5 +++++
>   2 files changed, 36 insertions(+), 16 deletions(-)
>
> diff --git a/ell/dbus-message.c b/ell/dbus-message.c
> index a409e53..86cec96 100644
> --- a/ell/dbus-message.c
> +++ b/ell/dbus-message.c
> @@ -328,33 +328,32 @@ LIB_EXPORT struct l_dbus_message *l_dbus_message_new_method_return(
>   	return message;
>   }
>
> -LIB_EXPORT struct l_dbus_message *l_dbus_message_new_error_valist(
> -					struct l_dbus_message *method_call,
> -					const char *name,
> -					const char *format, va_list args)
> +struct l_dbus_message *_dbus_message_new_error(uint8_t version,
> +						uint32_t reply_serial,
> +						const char *destination,
> +						const char *name,
> +						const char *error)
>   {
> -	char str[1024];
>   	struct l_dbus_message *reply;
> -	struct dbus_header *hdr = method_call->header;
> -	const char *sender;
> +	bool r;
>
>   	if (!_dbus_valid_interface(name))
>   		return NULL;
>
> -	vsnprintf(str, sizeof(str), format, args);
>   	reply = message_new_common(DBUS_MESSAGE_TYPE_ERROR,
>   					DBUS_MESSAGE_FLAG_NO_REPLY_EXPECTED,
> -					hdr->version);
> -
> -	reply->reply_serial = _dbus_message_get_serial(method_call);
> -
> -	sender = l_dbus_message_get_sender(method_call);
> -	if (sender)
> -		reply->destination = l_strdup(sender);
> +					version);
>
>   	reply->error_name = l_strdup(name);
> +	reply->destination = l_strdup(destination);
> +	reply->reply_serial = reply_serial;
>
> -	if (!l_dbus_message_set_arguments(reply, "s", str)) {
> +	if (error)
> +		r = l_dbus_message_set_arguments(reply, "s", error);
> +	else
> +		r = l_dbus_message_set_arguments(reply, "");

Under what circumstances would we want to create an error without at 
least a single string argument?

> +
> +	if (!r) {
>   		l_dbus_message_unref(reply);
>   		return NULL;
>   	}
> @@ -362,6 +361,22 @@ LIB_EXPORT struct l_dbus_message *l_dbus_message_new_error_valist(
>   	return reply;
>   }
>
> +LIB_EXPORT struct l_dbus_message *l_dbus_message_new_error_valist(
> +					struct l_dbus_message *method_call,
> +					const char *name,
> +					const char *format, va_list args)
> +{
> +	char str[1024];
> +	struct dbus_header *hdr = method_call->header;
> +
> +	vsnprintf(str, sizeof(str), format, args);
> +
> +	return _dbus_message_new_error(hdr->version,
> +					_dbus_message_get_serial(method_call),
> +					l_dbus_message_get_sender(method_call),
> +					name, str);
> +}
> +
>   LIB_EXPORT struct l_dbus_message *l_dbus_message_new_error(
>   					struct l_dbus_message *method_call,
>   					const char *name,
> diff --git a/ell/dbus-private.h b/ell/dbus-private.h
> index 63c239b..030ce0b 100644
> --- a/ell/dbus-private.h
> +++ b/ell/dbus-private.h
> @@ -141,6 +141,11 @@ struct l_dbus_message *_dbus_message_new_signal(uint8_t version,
>   						const char *path,
>   						const char *interface,
>   						const char *name);
> +struct l_dbus_message *_dbus_message_new_error(uint8_t version,
> +						uint32_t reply_serial,
> +						const char *destination,
> +						const char *name,
> +						const char *error);
>
>   struct l_dbus_message *dbus_message_from_blob(const void *data, size_t size);
>   struct l_dbus_message *dbus_message_build(void *header, size_t header_size,
>

Regards,
-Denis