2010-11-08 06:29:38

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 0/9] Fixing DBus error system in BlueZ

Hi,

Here are some patches that try to fix the mess of reporting error to
DBus inside BlueZ. It follows the oFono and ConnMan error system.

The goal is to get ride of any directly call to g_dbus_create_error()
inside bluez code, changing that to __btd_error_*. This patch set
doesn't fix all of them yet, but is a very good start. Please review.


Gustavo F. Padovan (9):
Create __btd_error_invalid_args()
Add __btd_error_already_exists()
Add __btd_error_not_supported()
Add __btd_error_not_connected()
Add __btd_error_in_progress()
Add __btd_error_not_available()
Add __btd_error_busy()
Add __btd_error_does_not_exist()
Add __btd_error_not_authorized()

attrib/client.c | 28 ++++-----------
audio/control.c | 16 ++-------
audio/device.c | 6 +--
audio/gateway.c | 16 ++-------
audio/headset.c | 58 ++++++++++-----------------------
audio/media.c | 12 ++----
audio/sink.c | 10 ++----
audio/source.c | 7 +---
audio/telephony-dummy.c | 25 ++++++--------
audio/telephony-maemo5.c | 11 ++----
audio/telephony-maemo6.c | 11 ++----
audio/transport.c | 14 ++------
health/hdp.c | 58 ++++++++------------------------
input/device.c | 8 +----
network/server.c | 9 +----
plugins/service.c | 56 ++++++++------------------------
serial/port.c | 20 +----------
serial/proxy.c | 31 ++++-------------
src/adapter.c | 81 +++++++++++++++++-----------------------------
src/device.c | 49 ++++++++--------------------
src/error.c | 62 +++++++++++++++++++++++++++++++++++
src/error.h | 10 ++++++
src/manager.c | 7 ----
23 files changed, 221 insertions(+), 384 deletions(-)

--
1.7.3.1



2010-11-10 15:38:36

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 0/9] Fixing DBus error system in BlueZ

Hi Marcel,

* Marcel Holtmann <[email protected]> [2010-11-10 14:23:42 +0900]:

> Hi Gustavo,
>
> > > On Mon, Nov 08, 2010, Gustavo F. Padovan wrote:
> > > > Here are some patches that try to fix the mess of reporting error to
> > > > DBus inside BlueZ. It follows the oFono and ConnMan error system.
> > > >
> > > > The goal is to get ride of any directly call to g_dbus_create_error()
> > > > inside bluez code, changing that to __btd_error_*. This patch set
> > > > doesn't fix all of them yet, but is a very good start. Please review.
> > > >
> > > >
> > > > Gustavo F. Padovan (9):
> > > > Create __btd_error_invalid_args()
> > > > Add __btd_error_already_exists()
> > > > Add __btd_error_not_supported()
> > > > Add __btd_error_not_connected()
> > > > Add __btd_error_in_progress()
> > > > Add __btd_error_not_available()
> > > > Add __btd_error_busy()
> > > > Add __btd_error_does_not_exist()
> > > > Add __btd_error_not_authorized()
> > >
> > > The patches seem fine to me, but before pushing upstream I'd like to
> > > understand the reason for prefixing these with with __btd instead of
> > > btd. What's the criteria used to decide what to use and when and why is
> > > __btd the correct choice for these new functions? My first guess would
> > > have been that __btd is for things only accessible by the core-daemon
> > > whereas btd is for functions exported to plugins, but that doesn't seem
> > > to be the case with your patches since many of these __btd functions get
> > > called from plugins.
> >
> > I just followed oFono and ConnMan on this. That is the reason and I
> > didn't asked myself why have a __ in this case.. But I see your point.
> > Do you think that change that to btd_error_* will fit better inside
> > BlueZ? I can change that then.
>
> so within ConnMan and oFono we make a difference between public symbols
> that are reachable from within plugins and other which are not.
>
> In general btd_ should be public symbols available to plugins and __btd_
> for internal symbols that are no available to plugins.
>
> For builtin plugins that makes no difference of course, but this is not
> about internal builtin plugins. It is for protecting against external
> plugins to not allow access to internal details.
>
> That said, bluetoothd is not linked with the case to hide certain
> symbols anyway so that right now there is no real difference here.

So to make that more clear, I'll change these functions to btd_

--
Gustavo F. Padovan
http://profusion.mobi

2010-11-10 05:23:42

by Marcel Holtmann

[permalink] [raw]
Subject: Re: [PATCH 0/9] Fixing DBus error system in BlueZ

Hi Gustavo,

> > On Mon, Nov 08, 2010, Gustavo F. Padovan wrote:
> > > Here are some patches that try to fix the mess of reporting error to
> > > DBus inside BlueZ. It follows the oFono and ConnMan error system.
> > >
> > > The goal is to get ride of any directly call to g_dbus_create_error()
> > > inside bluez code, changing that to __btd_error_*. This patch set
> > > doesn't fix all of them yet, but is a very good start. Please review.
> > >
> > >
> > > Gustavo F. Padovan (9):
> > > Create __btd_error_invalid_args()
> > > Add __btd_error_already_exists()
> > > Add __btd_error_not_supported()
> > > Add __btd_error_not_connected()
> > > Add __btd_error_in_progress()
> > > Add __btd_error_not_available()
> > > Add __btd_error_busy()
> > > Add __btd_error_does_not_exist()
> > > Add __btd_error_not_authorized()
> >
> > The patches seem fine to me, but before pushing upstream I'd like to
> > understand the reason for prefixing these with with __btd instead of
> > btd. What's the criteria used to decide what to use and when and why is
> > __btd the correct choice for these new functions? My first guess would
> > have been that __btd is for things only accessible by the core-daemon
> > whereas btd is for functions exported to plugins, but that doesn't seem
> > to be the case with your patches since many of these __btd functions get
> > called from plugins.
>
> I just followed oFono and ConnMan on this. That is the reason and I
> didn't asked myself why have a __ in this case.. But I see your point.
> Do you think that change that to btd_error_* will fit better inside
> BlueZ? I can change that then.

so within ConnMan and oFono we make a difference between public symbols
that are reachable from within plugins and other which are not.

In general btd_ should be public symbols available to plugins and __btd_
for internal symbols that are no available to plugins.

For builtin plugins that makes no difference of course, but this is not
about internal builtin plugins. It is for protecting against external
plugins to not allow access to internal details.

That said, bluetoothd is not linked with the case to hide certain
symbols anyway so that right now there is no real difference here.

Regards

Marcel



2010-11-10 00:51:49

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 1/9] Create __btd_error_invalid_args()

Hi Jose,

* Jose Antonio Santos Cadenas <[email protected]> [2010-11-09 09:20:33 +0100]:

> Hi Gustavo,
>
> 2010/11/8 Gustavo F. Padovan <[email protected]>:
> > DBus error handling in BlueZ is a mess. This is the first patch to unify
> > all DBus error handling like in ConnMan and oFono. This unifies all
> > .InvalidArguments errors.
> > ---
> > ?attrib/client.c ? ? ? ? ?| ? 20 +++++----------
> > ?audio/gateway.c ? ? ? ? ?| ? ?8 +----
> > ?audio/headset.c ? ? ? ? ?| ? 18 ++++---------
> > ?audio/media.c ? ? ? ? ? ?| ? ?9 ++----
> > ?audio/telephony-dummy.c ?| ? 25 ++++++++------------
> > ?audio/telephony-maemo5.c | ? 11 ++------
> > ?audio/telephony-maemo6.c | ? 11 ++------
> > ?audio/transport.c ? ? ? ?| ? 14 +++--------
> > ?health/hdp.c ? ? ? ? ? ? | ? 58 ++++++++++++----------------------------------
> > ?network/server.c ? ? ? ? | ? ?7 -----
> > ?plugins/service.c ? ? ? ?| ? ?8 +-----
> > ?serial/port.c ? ? ? ? ? ?| ? ?8 ------
> > ?serial/proxy.c ? ? ? ? ? | ? 19 ++++----------
> > ?src/adapter.c ? ? ? ? ? ?| ? 52 ++++++++++++++++++-----------------------
> > ?src/device.c ? ? ? ? ? ? | ? 22 ++++++-----------
> > ?src/error.c ? ? ? ? ? ? ?| ? ?7 +++++
> > ?src/error.h ? ? ? ? ? ? ?| ? ?2 +
> > ?src/manager.c ? ? ? ? ? ?| ? ?7 -----
> > ?18 files changed, 100 insertions(+), 206 deletions(-)
> >
> > diff --git a/attrib/client.c b/attrib/client.c
> > index 1f2c217..cd6e401 100644
> > --- a/attrib/client.c
> > +++ b/attrib/client.c
> > @@ -190,12 +190,6 @@ static int watcher_cmp(gconstpointer a, gconstpointer b)
> > ? ? ? ?return g_strcmp0(watcher->path, match->path);
> > ?}
> >
> > -static inline DBusMessage *invalid_args(DBusMessage *msg)
> > -{
> > - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> > -}
> > -
> > ?static inline DBusMessage *not_authorized(DBusMessage *msg)
> > ?{
> > ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
> > @@ -465,7 +459,7 @@ static DBusMessage *register_watcher(DBusConnection *conn,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (l2cap_connect(prim->gatt, &gerr, TRUE) < 0) {
> > ? ? ? ? ? ? ? ?DBusMessage *reply;
> > @@ -499,7 +493,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?match = g_new0(struct watcher, 1);
> > ? ? ? ?match->name = g_strdup(sender);
> > @@ -537,7 +531,7 @@ static DBusMessage *set_value(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY ||
> > ? ? ? ? ? ? ? ? ? ? ? ?dbus_message_iter_get_element_type(iter) != DBUS_TYPE_BYTE)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?dbus_message_iter_recurse(iter, &sub);
> >
> > @@ -586,23 +580,23 @@ static DBusMessage *set_property(DBusConnection *conn,
> > ? ? ? ?const char *property;
> >
> > ? ? ? ?if (!dbus_message_iter_init(msg, &iter))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?dbus_message_iter_get_basic(&iter, &property);
> > ? ? ? ?dbus_message_iter_next(&iter);
> >
> > ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?dbus_message_iter_recurse(&iter, &sub);
> >
> > ? ? ? ?if (g_str_equal("Value", property))
> > ? ? ? ? ? ? ? ?return set_value(conn, msg, &sub, chr);
> >
> > - ? ? ? return invalid_args(msg);
> > + ? ? ? return __btd_error_invalid_args(msg);
> > ?}
> >
> > ?static GDBusMethodTable char_methods[] = {
> > diff --git a/audio/gateway.c b/audio/gateway.c
> > index 07ebdd4..ab7d310 100644
> > --- a/audio/gateway.c
> > +++ b/audio/gateway.c
> > @@ -494,9 +494,7 @@ static DBusMessage *register_agent(DBusConnection *conn,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid argument");
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?name = dbus_message_get_sender(msg);
> > ? ? ? ?agent = g_new0(struct hf_agent, 1);
> > @@ -529,9 +527,7 @@ static DBusMessage *unregister_agent(DBusConnection *conn,
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_OBJECT_PATH, &path,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid argument");
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (strcmp(gw->agent->path, path) != 0)
> > ? ? ? ? ? ? ? ?return g_dbus_create_error(msg,
> > diff --git a/audio/headset.c b/audio/headset.c
> > index 0763585..2cca5ca 100644
> > --- a/audio/headset.c
> > +++ b/audio/headset.c
> > @@ -180,12 +180,6 @@ struct event {
> >
> > ?static GSList *headset_callbacks = NULL;
> >
> > -static inline DBusMessage *invalid_args(DBusMessage *msg)
> > -{
> > - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> > -}
> > -
> > ?static DBusHandlerResult error_not_supported(DBusConnection *conn,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBusMessage *msg)
> > ?{
> > @@ -2026,35 +2020,35 @@ static DBusMessage *hs_set_property(DBusConnection *conn,
> > ? ? ? ?uint16_t gain;
> >
> > ? ? ? ?if (!dbus_message_iter_init(msg, &iter))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?dbus_message_iter_get_basic(&iter, &property);
> > ? ? ? ?dbus_message_iter_next(&iter);
> >
> > ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> > ? ? ? ?dbus_message_iter_recurse(&iter, &sub);
> >
> > ? ? ? ?if (g_str_equal("SpeakerGain", property)) {
> > ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT16)
> > - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &gain);
> > ? ? ? ? ? ? ? ?return hs_set_gain(conn, msg, data, gain,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?HEADSET_GAIN_SPEAKER);
> > ? ? ? ?} else if (g_str_equal("MicrophoneGain", property)) {
> > ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT16)
> > - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &gain);
> > ? ? ? ? ? ? ? ?return hs_set_gain(conn, msg, data, gain,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?HEADSET_GAIN_MICROPHONE);
> > ? ? ? ?}
> >
> > - ? ? ? return invalid_args(msg);
> > + ? ? ? return __btd_error_invalid_args(msg);
> > ?}
> > ?static GDBusMethodTable headset_methods[] = {
> > ? ? ? ?{ "Connect", ? ? ? ? ? ?"", ? ? "", ? ? hs_connect,
> > diff --git a/audio/media.c b/audio/media.c
> > index b6c90f9..862cee6 100644
> > --- a/audio/media.c
> > +++ b/audio/media.c
> > @@ -323,18 +323,15 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?dbus_message_iter_recurse(&args, &props);
> > ? ? ? ?if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ".Failed", "Invalid argument");
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (parse_properties(&props, &uuid, &delay_reporting, &codec,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&capabilities, &size) || uuid == NULL)
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid argument");
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (media_endpoint_create(adapter, sender, path, uuid, delay_reporting,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?codec, capabilities, size) == FALSE)
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid argument");
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> > ?}
> > diff --git a/audio/telephony-dummy.c b/audio/telephony-dummy.c
> > index 06cb798..b56b6e7 100644
> > --- a/audio/telephony-dummy.c
> > +++ b/audio/telephony-dummy.c
> > @@ -35,6 +35,7 @@
> >
> > ?#include "log.h"
> > ?#include "telephony.h"
> > +#include "error.h"
> >
> > ?#define TELEPHONY_DUMMY_IFACE "org.bluez.TelephonyTest"
> > ?#define TELEPHONY_DUMMY_PATH "/org/bluez/test"
> > @@ -69,12 +70,6 @@ static struct indicator dummy_indicators[] =
> > ? ? ? ?{ NULL }
> > ?};
> >
> > -static inline DBusMessage *invalid_args(DBusMessage *msg)
> > -{
> > - ? ? ? return g_dbus_create_error(msg, "org.bluez.Error.InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> > -}
> > -
> > ?void telephony_device_connected(void *telephony_device)
> > ?{
> > ? ? ? ?DBG("telephony-dummy: device %p connected", telephony_device);
> > @@ -236,7 +231,7 @@ static DBusMessage *outgoing_call(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?DBG("telephony-dummy: outgoing call to %s", number);
> >
> > @@ -261,7 +256,7 @@ static DBusMessage *incoming_call(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?DBG("telephony-dummy: incoming call to %s", number);
> >
> > @@ -307,10 +302,10 @@ static DBusMessage *signal_strength(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UINT32, &strength,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (strength > 5)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?telephony_update_indicator(dummy_indicators, "signal", strength);
> >
> > @@ -326,10 +321,10 @@ static DBusMessage *battery_level(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UINT32, &level,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (level > 5)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?telephony_update_indicator(dummy_indicators, "battchg", level);
> >
> > @@ -346,7 +341,7 @@ static DBusMessage *roaming_status(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_BOOLEAN, &roaming,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?val = roaming ? EV_ROAM_ACTIVE : EV_ROAM_INACTIVE;
> >
> > @@ -365,7 +360,7 @@ static DBusMessage *registration_status(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_BOOLEAN, &registration,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?val = registration ? EV_SERVICE_PRESENT : EV_SERVICE_NONE;
> >
> > @@ -384,7 +379,7 @@ static DBusMessage *set_subscriber_number(DBusConnection *conn,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?g_free(subscriber_number);
> > ? ? ? ?subscriber_number = g_strdup(number);
> > diff --git a/audio/telephony-maemo5.c b/audio/telephony-maemo5.c
> > index 4d0134c..0f9819c 100644
> > --- a/audio/telephony-maemo5.c
> > +++ b/audio/telephony-maemo5.c
> > @@ -38,6 +38,7 @@
> >
> > ?#include "log.h"
> > ?#include "telephony.h"
> > +#include "error.h"
> >
> > ?/* SSC D-Bus definitions */
> > ?#define SSC_DBUS_NAME ?"com.nokia.phone.SSC"
> > @@ -1880,12 +1881,6 @@ static void csd_init(void)
> > ? ? ? ?}
> > ?}
> >
> > -static inline DBusMessage *invalid_args(DBusMessage *msg)
> > -{
> > - ? ? ? return g_dbus_create_error(msg,"org.bluez.Error.InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> > -}
> > -
> > ?static uint32_t get_callflag(const char *callerid_setting)
> > ?{
> > ? ? ? ?if (callerid_setting != NULL) {
> > @@ -1950,7 +1945,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
> > ? ? ? ?if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&callerid_setting,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID) == FALSE)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (g_str_equal(callerid_setting, "allowed") ||
> > ? ? ? ? ? ? ? ? ? ? ? ?g_str_equal(callerid_setting, "restricted") ||
> > @@ -1964,7 +1959,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?error("telephony-maemo: invalid argument %s for method call"
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?" SetCallerId", callerid_setting);
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> > ?}
> >
> > ?static GDBusMethodTable telephony_maemo_methods[] = {
> > diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
> > index 72c8e36..4663b4d 100644
> > --- a/audio/telephony-maemo6.c
> > +++ b/audio/telephony-maemo6.c
> > @@ -38,6 +38,7 @@
> >
> > ?#include "log.h"
> > ?#include "telephony.h"
> > +#include "error.h"
> >
> > ?/* SSC D-Bus definitions */
> > ?#define SSC_DBUS_NAME ?"com.nokia.phone.SSC"
> > @@ -1760,12 +1761,6 @@ static void csd_init(void)
> > ? ? ? ?}
> > ?}
> >
> > -static inline DBusMessage *invalid_args(DBusMessage *msg)
> > -{
> > - ? ? ? return g_dbus_create_error(msg,"org.bluez.Error.InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> > -}
> > -
> > ?static uint32_t get_callflag(const char *callerid_setting)
> > ?{
> > ? ? ? ?if (callerid_setting != NULL) {
> > @@ -1830,7 +1825,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
> > ? ? ? ?if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&callerid_setting,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID) == FALSE)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (g_str_equal(callerid_setting, "allowed") ||
> > ? ? ? ? ? ? ? ? ? ? ? ?g_str_equal(callerid_setting, "restricted") ||
> > @@ -1844,7 +1839,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?error("telephony-maemo6: invalid argument %s for method call"
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?" SetCallerId", callerid_setting);
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> > ?}
> >
> > ?static DBusMessage *clear_lastnumber(DBusConnection *conn, DBusMessage *msg,
> > diff --git a/audio/transport.c b/audio/transport.c
> > index eda46e1..0c865f7 100644
> > --- a/audio/transport.c
> > +++ b/audio/transport.c
> > @@ -93,12 +93,6 @@ struct media_transport {
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBusMessageIter *value);
> > ?};
> >
> > -static inline DBusMessage *invalid_args(DBusMessage *msg)
> > -{
> > - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> > -}
> > -
> > ?static inline DBusMessage *error_failed(DBusMessage *msg, const char *desc)
> > ?{
> > ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "%s", desc);
> > @@ -549,16 +543,16 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
> > ? ? ? ?int err;
> >
> > ? ? ? ?if (!dbus_message_iter_init(msg, &iter))
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?dbus_message_iter_get_basic(&iter, &property);
> > ? ? ? ?dbus_message_iter_next(&iter);
> >
> > ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> > - ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> > ? ? ? ?dbus_message_iter_recurse(&iter, &value);
> >
> > ? ? ? ?sender = dbus_message_get_sender(msg);
> > @@ -577,7 +571,7 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
> >
> > ? ? ? ?if (err < 0) {
> > ? ? ? ? ? ? ? ?if (err == -EINVAL)
> > - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> > + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> > ? ? ? ? ? ? ? ?return error_failed(msg, strerror(-err));
> > ? ? ? ?}
> >
> > diff --git a/health/hdp.c b/health/hdp.c
> > index 1eba8e1..6c1fd98 100644
> > --- a/health/hdp.c
> > +++ b/health/hdp.c
> > @@ -321,15 +321,8 @@ static DBusMessage *manager_create_application(DBusConnection *conn,
> >
> > ? ? ? ?dbus_message_iter_init(msg, &iter);
> > ? ? ? ?app = hdp_get_app_config(&iter, &err);
> > - ? ? ? if (err) {
> > - ? ? ? ? ? ? ? DBusMessage *reply;
> > -
> > - ? ? ? ? ? ? ? reply = g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments: %s", err->message);
> > - ? ? ? ? ? ? ? g_error_free(err);
> > - ? ? ? ? ? ? ? return reply;
> > - ? ? ? }
> > + ? ? ? if (err)
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
>
> You are leaking memory here, err should be freed before return. Also
> the message that we add to the error is for clarify the user the kind
> of error, it will be great to keep it.

Sure.

>
> >
> > ? ? ? ?name = dbus_message_get_sender(msg);
> > ? ? ? ?if (!name) {
> > @@ -368,11 +361,8 @@ static DBusMessage *manager_destroy_application(DBusConnection *conn,
> > ? ? ? ?GSList *l;
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DBUS_TYPE_INVALID)){
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> > - ? ? ? }
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DBUS_TYPE_INVALID))
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?l = g_slist_find_custom(applications, path, cmp_app);
> >
> > @@ -1801,18 +1791,13 @@ static DBusMessage *device_create_channel(DBusConnection *conn,
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &app_path,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_STRING, &conf,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DBUS_TYPE_INVALID)) {
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> > - ? ? ? }
> > + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DBUS_TYPE_INVALID))
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?l = g_slist_find_custom(applications, app_path, cmp_app);
> > ? ? ? ?if (!l)
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call, "
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "no such application");
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> > +
> > ? ? ? ?app = l->data;
> >
> > ? ? ? ?if (g_ascii_strcasecmp("Reliable", conf) == 0)
> > @@ -1822,25 +1807,16 @@ static DBusMessage *device_create_channel(DBusConnection *conn,
> > ? ? ? ?else if (g_ascii_strcasecmp("Any", conf) == 0)
> > ? ? ? ? ? ? ? ?config = HDP_NO_PREFERENCE_DC;
> > ? ? ? ?else
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (app->role == HDP_SINK && config != HDP_NO_PREFERENCE_DC)
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Configuration not valid for sinks");
> > + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> Also here the message we tried to clarify the kind of "Invalid
> Arguments" error with a different message.

I'm not really caring about those messages, I'm fine on removing all of
them. The idea is to just have a __btd_error_failed_str() to report
string messages in some more generic errors.


Subject: Re: [PATCH 2/9] Add __btd_error_already_exists()

Hi,

2010/11/9 Gustavo F. Padovan <[email protected]>:
> HI Jose,
>
> * Jose Antonio Santos Cadenas <[email protected]> [2010-11-09 21:13:57 +0100]:
>
>> Hi Gustavo,
>>
>> 2010/11/9 Gustavo F. Padovan <[email protected]>:
>> > Hi Jose,
>> >
>> > * Jose Antonio Santos Cadenas <[email protected]> [2010-11-09 09:22:56 +0100]:
>> >
>> >> Hi,
>> >>
>> >> 2010/11/8 Gustavo F. Padovan <[email protected]>:
>> >> > ---
>> >> > ?audio/gateway.c ?| ? ?4 +---
>> >> > ?audio/media.c ? ?| ? ?3 +--
>> >> > ?network/server.c | ? ?2 +-
>> >> > ?serial/proxy.c ? | ? ?3 +--
>> >> > ?src/adapter.c ? ?| ? ?8 ++------
>> >> > ?src/device.c ? ? | ? ?4 +---
>> >> > ?src/error.c ? ? ?| ? ?7 +++++++
>> >> > ?src/error.h ? ? ?| ? ?1 +
>> >> > ?8 files changed, 15 insertions(+), 17 deletions(-)
>> >> >
>> >> > diff --git a/audio/gateway.c b/audio/gateway.c
>> >> > index ab7d310..ae0ee75 100644
>> >> > --- a/audio/gateway.c
>> >> > +++ b/audio/gateway.c
>> >> > @@ -488,9 +488,7 @@ static DBusMessage *register_agent(DBusConnection *conn,
>> >> > ? ? ? ?const char *path, *name;
>> >> >
>> >> > ? ? ? ?if (gw->agent)
>> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Agent already exists");
>> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >> >
>> >> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
>> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
>> >> > diff --git a/audio/media.c b/audio/media.c
>> >> > index 862cee6..8821ee1 100644
>> >> > --- a/audio/media.c
>> >> > +++ b/audio/media.c
>> >> > @@ -318,8 +318,7 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,
>> >> > ? ? ? ?dbus_message_iter_next(&args);
>> >> >
>> >> > ? ? ? ?if (media_adapter_find_endpoint(adapter, sender, path, NULL) != NULL)
>> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Endpoint already registered");
>> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >> >
>> >> > ? ? ? ?dbus_message_iter_recurse(&args, &props);
>> >> > ? ? ? ?if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
>> >> > diff --git a/network/server.c b/network/server.c
>> >> > index ce1fe5e..41c9ec3 100644
>> >> > --- a/network/server.c
>> >> > +++ b/network/server.c
>> >> > @@ -605,7 +605,7 @@ static DBusMessage *register_server(DBusConnection *conn,
>> >> > ? ? ? ? ? ? ? ?return failed(msg, "Invalid UUID");
>> >> >
>> >> > ? ? ? ?if (ns->record_id)
>> >> > - ? ? ? ? ? ? ? return failed(msg, "Already registered");
>> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >> >
>> >> > ? ? ? ?reply = dbus_message_new_method_return(msg);
>> >> > ? ? ? ?if (!reply)
>> >> > diff --git a/serial/proxy.c b/serial/proxy.c
>> >> > index 8e182b6..de82f9a 100644
>> >> > --- a/serial/proxy.c
>> >> > +++ b/serial/proxy.c
>> >> > @@ -1058,8 +1058,7 @@ static DBusMessage *create_proxy(DBusConnection *conn,
>> >> > ? ? ? ?if (err == -EINVAL)
>> >> > ? ? ? ? ? ? ? ?return __btd_error_invalid_args(msg);
>> >> > ? ? ? ?else if (err == -EALREADY)
>> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExist",
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Proxy already exists");
>> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >> > ? ? ? ?else if (err < 0)
>> >> > ? ? ? ? ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE "Failed",
>> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Proxy creation failed (%s)", strerror(-err));
>> >> > diff --git a/src/adapter.c b/src/adapter.c
>> >> > index cc51816..ffbc943 100644
>> >> > --- a/src/adapter.c
>> >> > +++ b/src/adapter.c
>> >> > @@ -1742,9 +1742,7 @@ static DBusMessage *create_device(DBusConnection *conn,
>> >> > ? ? ? ? ? ? ? ?return __btd_error_invalid_args(msg);
>> >> >
>> >> > ? ? ? ?if (adapter_find_device(adapter, address))
>> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Device already exists");
>> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >> >
>> >> > ? ? ? ?DBG("%s", address);
>> >> >
>> >> > @@ -1906,9 +1904,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
>> >> > ? ? ? ? ? ? ? ?return NULL;
>> >> >
>> >> > ? ? ? ?if (adapter->agent)
>> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Agent already exists");
>> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >> >
>> >> > ? ? ? ?cap = parse_io_capability(capability);
>> >> > ? ? ? ?if (cap == IO_CAPABILITY_INVALID)
>> >> > diff --git a/src/device.c b/src/device.c
>> >> > index ef1a910..6d110dc 100644
>> >> > --- a/src/device.c
>> >> > +++ b/src/device.c
>> >> > @@ -1944,9 +1944,7 @@ DBusMessage *device_create_bonding(struct btd_device *device,
>> >> > ? ? ? ?str = textfile_caseget(filename, dstaddr);
>> >> > ? ? ? ?if (str) {
>> >> > ? ? ? ? ? ? ? ?free(str);
>> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
>> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Bonding already exists");
>> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >> > ? ? ? ?}
>> >> >
>> >> > ? ? ? ?/* If our IO capability is NoInputNoOutput use medium security
>> >> > diff --git a/src/error.c b/src/error.c
>> >> > index a30c050..e268163 100644
>> >> > --- a/src/error.c
>> >> > +++ b/src/error.c
>> >> > @@ -55,3 +55,10 @@ DBusMessage *__btd_error_invalid_args(DBusMessage *msg)
>> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?".InvalidArguments",
>> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Invalid arguments in method call");
>> >> > ?}
>> >> > +
>> >> > +DBusMessage *__btd_error_already_exists(DBusMessage *msg)
>> >>
>> >> I also think that an additional message will be great for a better description.
>> >
>> > I'm not seeing any case when we need that for already_exists(). All
>> > messages were saying the same thing. We don't need them.
>>
>>
>> The messages are different for each case: (ie: "Bonding already
>> exists", "Agent already exists", etc), they are nearly the same but
>> they are a little bit more expressive. Also if we keep the additional
>> message, we are following the same behaviour than in the other error
>> funtions, this way this API is easy to use.
>
> Yes, they are, but you always know the DBus funcion you called. If you
> call RegisterAgent() and it replies .AlreadyExists you will know that is
> a Agent that already exists, and not a Bonding, Device, etc. So no need
> to explain the error in theses cases.

You are right, in most of the cases the message won't be needed or
won't give extra information. I just see the uniformity simpler (just
my opinion).

>
>

2010-11-09 20:30:58

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 2/9] Add __btd_error_already_exists()

HI Jose,

* Jose Antonio Santos Cadenas <[email protected]> [2010-11-09 21:13:57 +0100]:

> Hi Gustavo,
>
> 2010/11/9 Gustavo F. Padovan <[email protected]>:
> > Hi Jose,
> >
> > * Jose Antonio Santos Cadenas <[email protected]> [2010-11-09 09:22:56 +0100]:
> >
> >> Hi,
> >>
> >> 2010/11/8 Gustavo F. Padovan <[email protected]>:
> >> > ---
> >> > ?audio/gateway.c ?| ? ?4 +---
> >> > ?audio/media.c ? ?| ? ?3 +--
> >> > ?network/server.c | ? ?2 +-
> >> > ?serial/proxy.c ? | ? ?3 +--
> >> > ?src/adapter.c ? ?| ? ?8 ++------
> >> > ?src/device.c ? ? | ? ?4 +---
> >> > ?src/error.c ? ? ?| ? ?7 +++++++
> >> > ?src/error.h ? ? ?| ? ?1 +
> >> > ?8 files changed, 15 insertions(+), 17 deletions(-)
> >> >
> >> > diff --git a/audio/gateway.c b/audio/gateway.c
> >> > index ab7d310..ae0ee75 100644
> >> > --- a/audio/gateway.c
> >> > +++ b/audio/gateway.c
> >> > @@ -488,9 +488,7 @@ static DBusMessage *register_agent(DBusConnection *conn,
> >> > ? ? ? ?const char *path, *name;
> >> >
> >> > ? ? ? ?if (gw->agent)
> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Agent already exists");
> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >> >
> >> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> >> > diff --git a/audio/media.c b/audio/media.c
> >> > index 862cee6..8821ee1 100644
> >> > --- a/audio/media.c
> >> > +++ b/audio/media.c
> >> > @@ -318,8 +318,7 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,
> >> > ? ? ? ?dbus_message_iter_next(&args);
> >> >
> >> > ? ? ? ?if (media_adapter_find_endpoint(adapter, sender, path, NULL) != NULL)
> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Endpoint already registered");
> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >> >
> >> > ? ? ? ?dbus_message_iter_recurse(&args, &props);
> >> > ? ? ? ?if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
> >> > diff --git a/network/server.c b/network/server.c
> >> > index ce1fe5e..41c9ec3 100644
> >> > --- a/network/server.c
> >> > +++ b/network/server.c
> >> > @@ -605,7 +605,7 @@ static DBusMessage *register_server(DBusConnection *conn,
> >> > ? ? ? ? ? ? ? ?return failed(msg, "Invalid UUID");
> >> >
> >> > ? ? ? ?if (ns->record_id)
> >> > - ? ? ? ? ? ? ? return failed(msg, "Already registered");
> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >> >
> >> > ? ? ? ?reply = dbus_message_new_method_return(msg);
> >> > ? ? ? ?if (!reply)
> >> > diff --git a/serial/proxy.c b/serial/proxy.c
> >> > index 8e182b6..de82f9a 100644
> >> > --- a/serial/proxy.c
> >> > +++ b/serial/proxy.c
> >> > @@ -1058,8 +1058,7 @@ static DBusMessage *create_proxy(DBusConnection *conn,
> >> > ? ? ? ?if (err == -EINVAL)
> >> > ? ? ? ? ? ? ? ?return __btd_error_invalid_args(msg);
> >> > ? ? ? ?else if (err == -EALREADY)
> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExist",
> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Proxy already exists");
> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >> > ? ? ? ?else if (err < 0)
> >> > ? ? ? ? ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE "Failed",
> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Proxy creation failed (%s)", strerror(-err));
> >> > diff --git a/src/adapter.c b/src/adapter.c
> >> > index cc51816..ffbc943 100644
> >> > --- a/src/adapter.c
> >> > +++ b/src/adapter.c
> >> > @@ -1742,9 +1742,7 @@ static DBusMessage *create_device(DBusConnection *conn,
> >> > ? ? ? ? ? ? ? ?return __btd_error_invalid_args(msg);
> >> >
> >> > ? ? ? ?if (adapter_find_device(adapter, address))
> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Device already exists");
> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >> >
> >> > ? ? ? ?DBG("%s", address);
> >> >
> >> > @@ -1906,9 +1904,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
> >> > ? ? ? ? ? ? ? ?return NULL;
> >> >
> >> > ? ? ? ?if (adapter->agent)
> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Agent already exists");
> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >> >
> >> > ? ? ? ?cap = parse_io_capability(capability);
> >> > ? ? ? ?if (cap == IO_CAPABILITY_INVALID)
> >> > diff --git a/src/device.c b/src/device.c
> >> > index ef1a910..6d110dc 100644
> >> > --- a/src/device.c
> >> > +++ b/src/device.c
> >> > @@ -1944,9 +1944,7 @@ DBusMessage *device_create_bonding(struct btd_device *device,
> >> > ? ? ? ?str = textfile_caseget(filename, dstaddr);
> >> > ? ? ? ?if (str) {
> >> > ? ? ? ? ? ? ? ?free(str);
> >> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> >> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Bonding already exists");
> >> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >> > ? ? ? ?}
> >> >
> >> > ? ? ? ?/* If our IO capability is NoInputNoOutput use medium security
> >> > diff --git a/src/error.c b/src/error.c
> >> > index a30c050..e268163 100644
> >> > --- a/src/error.c
> >> > +++ b/src/error.c
> >> > @@ -55,3 +55,10 @@ DBusMessage *__btd_error_invalid_args(DBusMessage *msg)
> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?".InvalidArguments",
> >> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Invalid arguments in method call");
> >> > ?}
> >> > +
> >> > +DBusMessage *__btd_error_already_exists(DBusMessage *msg)
> >>
> >> I also think that an additional message will be great for a better description.
> >
> > I'm not seeing any case when we need that for already_exists(). All
> > messages were saying the same thing. We don't need them.
>
>
> The messages are different for each case: (ie: "Bonding already
> exists", "Agent already exists", etc), they are nearly the same but
> they are a little bit more expressive. Also if we keep the additional
> message, we are following the same behaviour than in the other error
> funtions, this way this API is easy to use.

Yes, they are, but you always know the DBus funcion you called. If you
call RegisterAgent() and it replies .AlreadyExists you will know that is
a Agent that already exists, and not a Bonding, Device, etc. So no need
to explain the error in theses cases.


Subject: Re: [PATCH 2/9] Add __btd_error_already_exists()

Hi Gustavo,

2010/11/9 Gustavo F. Padovan <[email protected]>:
> Hi Jose,
>
> * Jose Antonio Santos Cadenas <[email protected]> [2010-11-09 09:22:56 +0100]:
>
>> Hi,
>>
>> 2010/11/8 Gustavo F. Padovan <[email protected]>:
>> > ---
>> > ?audio/gateway.c ?| ? ?4 +---
>> > ?audio/media.c ? ?| ? ?3 +--
>> > ?network/server.c | ? ?2 +-
>> > ?serial/proxy.c ? | ? ?3 +--
>> > ?src/adapter.c ? ?| ? ?8 ++------
>> > ?src/device.c ? ? | ? ?4 +---
>> > ?src/error.c ? ? ?| ? ?7 +++++++
>> > ?src/error.h ? ? ?| ? ?1 +
>> > ?8 files changed, 15 insertions(+), 17 deletions(-)
>> >
>> > diff --git a/audio/gateway.c b/audio/gateway.c
>> > index ab7d310..ae0ee75 100644
>> > --- a/audio/gateway.c
>> > +++ b/audio/gateway.c
>> > @@ -488,9 +488,7 @@ static DBusMessage *register_agent(DBusConnection *conn,
>> > ? ? ? ?const char *path, *name;
>> >
>> > ? ? ? ?if (gw->agent)
>> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Agent already exists");
>> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >
>> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
>> > diff --git a/audio/media.c b/audio/media.c
>> > index 862cee6..8821ee1 100644
>> > --- a/audio/media.c
>> > +++ b/audio/media.c
>> > @@ -318,8 +318,7 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,
>> > ? ? ? ?dbus_message_iter_next(&args);
>> >
>> > ? ? ? ?if (media_adapter_find_endpoint(adapter, sender, path, NULL) != NULL)
>> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Endpoint already registered");
>> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >
>> > ? ? ? ?dbus_message_iter_recurse(&args, &props);
>> > ? ? ? ?if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
>> > diff --git a/network/server.c b/network/server.c
>> > index ce1fe5e..41c9ec3 100644
>> > --- a/network/server.c
>> > +++ b/network/server.c
>> > @@ -605,7 +605,7 @@ static DBusMessage *register_server(DBusConnection *conn,
>> > ? ? ? ? ? ? ? ?return failed(msg, "Invalid UUID");
>> >
>> > ? ? ? ?if (ns->record_id)
>> > - ? ? ? ? ? ? ? return failed(msg, "Already registered");
>> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >
>> > ? ? ? ?reply = dbus_message_new_method_return(msg);
>> > ? ? ? ?if (!reply)
>> > diff --git a/serial/proxy.c b/serial/proxy.c
>> > index 8e182b6..de82f9a 100644
>> > --- a/serial/proxy.c
>> > +++ b/serial/proxy.c
>> > @@ -1058,8 +1058,7 @@ static DBusMessage *create_proxy(DBusConnection *conn,
>> > ? ? ? ?if (err == -EINVAL)
>> > ? ? ? ? ? ? ? ?return __btd_error_invalid_args(msg);
>> > ? ? ? ?else if (err == -EALREADY)
>> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExist",
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Proxy already exists");
>> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> > ? ? ? ?else if (err < 0)
>> > ? ? ? ? ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE "Failed",
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Proxy creation failed (%s)", strerror(-err));
>> > diff --git a/src/adapter.c b/src/adapter.c
>> > index cc51816..ffbc943 100644
>> > --- a/src/adapter.c
>> > +++ b/src/adapter.c
>> > @@ -1742,9 +1742,7 @@ static DBusMessage *create_device(DBusConnection *conn,
>> > ? ? ? ? ? ? ? ?return __btd_error_invalid_args(msg);
>> >
>> > ? ? ? ?if (adapter_find_device(adapter, address))
>> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Device already exists");
>> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >
>> > ? ? ? ?DBG("%s", address);
>> >
>> > @@ -1906,9 +1904,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
>> > ? ? ? ? ? ? ? ?return NULL;
>> >
>> > ? ? ? ?if (adapter->agent)
>> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Agent already exists");
>> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> >
>> > ? ? ? ?cap = parse_io_capability(capability);
>> > ? ? ? ?if (cap == IO_CAPABILITY_INVALID)
>> > diff --git a/src/device.c b/src/device.c
>> > index ef1a910..6d110dc 100644
>> > --- a/src/device.c
>> > +++ b/src/device.c
>> > @@ -1944,9 +1944,7 @@ DBusMessage *device_create_bonding(struct btd_device *device,
>> > ? ? ? ?str = textfile_caseget(filename, dstaddr);
>> > ? ? ? ?if (str) {
>> > ? ? ? ? ? ? ? ?free(str);
>> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
>> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Bonding already exists");
>> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>> > ? ? ? ?}
>> >
>> > ? ? ? ?/* If our IO capability is NoInputNoOutput use medium security
>> > diff --git a/src/error.c b/src/error.c
>> > index a30c050..e268163 100644
>> > --- a/src/error.c
>> > +++ b/src/error.c
>> > @@ -55,3 +55,10 @@ DBusMessage *__btd_error_invalid_args(DBusMessage *msg)
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?".InvalidArguments",
>> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Invalid arguments in method call");
>> > ?}
>> > +
>> > +DBusMessage *__btd_error_already_exists(DBusMessage *msg)
>>
>> I also think that an additional message will be great for a better description.
>
> I'm not seeing any case when we need that for already_exists(). All
> messages were saying the same thing. We don't need them.


The messages are different for each case: (ie: "Bonding already
exists", "Agent already exists", etc), they are nearly the same but
they are a little bit more expressive. Also if we keep the additional
message, we are following the same behaviour than in the other error
funtions, this way this API is easy to use.

Regards.

>
> --
> Gustavo F. Padovan
> ProFUSION embedded systems - http://profusion.mobi
>

2010-11-09 17:13:17

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 2/9] Add __btd_error_already_exists()

Hi Jose,

* Jose Antonio Santos Cadenas <[email protected]> [2010-11-09 09:22:56 +0100]:

> Hi,
>
> 2010/11/8 Gustavo F. Padovan <[email protected]>:
> > ---
> > ?audio/gateway.c ?| ? ?4 +---
> > ?audio/media.c ? ?| ? ?3 +--
> > ?network/server.c | ? ?2 +-
> > ?serial/proxy.c ? | ? ?3 +--
> > ?src/adapter.c ? ?| ? ?8 ++------
> > ?src/device.c ? ? | ? ?4 +---
> > ?src/error.c ? ? ?| ? ?7 +++++++
> > ?src/error.h ? ? ?| ? ?1 +
> > ?8 files changed, 15 insertions(+), 17 deletions(-)
> >
> > diff --git a/audio/gateway.c b/audio/gateway.c
> > index ab7d310..ae0ee75 100644
> > --- a/audio/gateway.c
> > +++ b/audio/gateway.c
> > @@ -488,9 +488,7 @@ static DBusMessage *register_agent(DBusConnection *conn,
> > ? ? ? ?const char *path, *name;
> >
> > ? ? ? ?if (gw->agent)
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Agent already exists");
> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >
> > ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> > diff --git a/audio/media.c b/audio/media.c
> > index 862cee6..8821ee1 100644
> > --- a/audio/media.c
> > +++ b/audio/media.c
> > @@ -318,8 +318,7 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,
> > ? ? ? ?dbus_message_iter_next(&args);
> >
> > ? ? ? ?if (media_adapter_find_endpoint(adapter, sender, path, NULL) != NULL)
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Endpoint already registered");
> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >
> > ? ? ? ?dbus_message_iter_recurse(&args, &props);
> > ? ? ? ?if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
> > diff --git a/network/server.c b/network/server.c
> > index ce1fe5e..41c9ec3 100644
> > --- a/network/server.c
> > +++ b/network/server.c
> > @@ -605,7 +605,7 @@ static DBusMessage *register_server(DBusConnection *conn,
> > ? ? ? ? ? ? ? ?return failed(msg, "Invalid UUID");
> >
> > ? ? ? ?if (ns->record_id)
> > - ? ? ? ? ? ? ? return failed(msg, "Already registered");
> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >
> > ? ? ? ?reply = dbus_message_new_method_return(msg);
> > ? ? ? ?if (!reply)
> > diff --git a/serial/proxy.c b/serial/proxy.c
> > index 8e182b6..de82f9a 100644
> > --- a/serial/proxy.c
> > +++ b/serial/proxy.c
> > @@ -1058,8 +1058,7 @@ static DBusMessage *create_proxy(DBusConnection *conn,
> > ? ? ? ?if (err == -EINVAL)
> > ? ? ? ? ? ? ? ?return __btd_error_invalid_args(msg);
> > ? ? ? ?else if (err == -EALREADY)
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExist",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Proxy already exists");
> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> > ? ? ? ?else if (err < 0)
> > ? ? ? ? ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE "Failed",
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Proxy creation failed (%s)", strerror(-err));
> > diff --git a/src/adapter.c b/src/adapter.c
> > index cc51816..ffbc943 100644
> > --- a/src/adapter.c
> > +++ b/src/adapter.c
> > @@ -1742,9 +1742,7 @@ static DBusMessage *create_device(DBusConnection *conn,
> > ? ? ? ? ? ? ? ?return __btd_error_invalid_args(msg);
> >
> > ? ? ? ?if (adapter_find_device(adapter, address))
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Device already exists");
> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >
> > ? ? ? ?DBG("%s", address);
> >
> > @@ -1906,9 +1904,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
> > ? ? ? ? ? ? ? ?return NULL;
> >
> > ? ? ? ?if (adapter->agent)
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Agent already exists");
> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> >
> > ? ? ? ?cap = parse_io_capability(capability);
> > ? ? ? ?if (cap == IO_CAPABILITY_INVALID)
> > diff --git a/src/device.c b/src/device.c
> > index ef1a910..6d110dc 100644
> > --- a/src/device.c
> > +++ b/src/device.c
> > @@ -1944,9 +1944,7 @@ DBusMessage *device_create_bonding(struct btd_device *device,
> > ? ? ? ?str = textfile_caseget(filename, dstaddr);
> > ? ? ? ?if (str) {
> > ? ? ? ? ? ? ? ?free(str);
> > - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> > - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Bonding already exists");
> > + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> > ? ? ? ?}
> >
> > ? ? ? ?/* If our IO capability is NoInputNoOutput use medium security
> > diff --git a/src/error.c b/src/error.c
> > index a30c050..e268163 100644
> > --- a/src/error.c
> > +++ b/src/error.c
> > @@ -55,3 +55,10 @@ DBusMessage *__btd_error_invalid_args(DBusMessage *msg)
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?".InvalidArguments",
> > ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Invalid arguments in method call");
> > ?}
> > +
> > +DBusMessage *__btd_error_already_exists(DBusMessage *msg)
>
> I also think that an additional message will be great for a better description.

I'm not seeing any case when we need that for already_exists(). All
messages were saying the same thing. We don't need them.

--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

Subject: Re: [PATCH 2/9] Add __btd_error_already_exists()

Hi,

2010/11/8 Gustavo F. Padovan <[email protected]>:
> ---
> ?audio/gateway.c ?| ? ?4 +---
> ?audio/media.c ? ?| ? ?3 +--
> ?network/server.c | ? ?2 +-
> ?serial/proxy.c ? | ? ?3 +--
> ?src/adapter.c ? ?| ? ?8 ++------
> ?src/device.c ? ? | ? ?4 +---
> ?src/error.c ? ? ?| ? ?7 +++++++
> ?src/error.h ? ? ?| ? ?1 +
> ?8 files changed, 15 insertions(+), 17 deletions(-)
>
> diff --git a/audio/gateway.c b/audio/gateway.c
> index ab7d310..ae0ee75 100644
> --- a/audio/gateway.c
> +++ b/audio/gateway.c
> @@ -488,9 +488,7 @@ static DBusMessage *register_agent(DBusConnection *conn,
> ? ? ? ?const char *path, *name;
>
> ? ? ? ?if (gw->agent)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Agent already exists");
> + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> diff --git a/audio/media.c b/audio/media.c
> index 862cee6..8821ee1 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -318,8 +318,7 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,
> ? ? ? ?dbus_message_iter_next(&args);
>
> ? ? ? ?if (media_adapter_find_endpoint(adapter, sender, path, NULL) != NULL)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Endpoint already registered");
> + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>
> ? ? ? ?dbus_message_iter_recurse(&args, &props);
> ? ? ? ?if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
> diff --git a/network/server.c b/network/server.c
> index ce1fe5e..41c9ec3 100644
> --- a/network/server.c
> +++ b/network/server.c
> @@ -605,7 +605,7 @@ static DBusMessage *register_server(DBusConnection *conn,
> ? ? ? ? ? ? ? ?return failed(msg, "Invalid UUID");
>
> ? ? ? ?if (ns->record_id)
> - ? ? ? ? ? ? ? return failed(msg, "Already registered");
> + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>
> ? ? ? ?reply = dbus_message_new_method_return(msg);
> ? ? ? ?if (!reply)
> diff --git a/serial/proxy.c b/serial/proxy.c
> index 8e182b6..de82f9a 100644
> --- a/serial/proxy.c
> +++ b/serial/proxy.c
> @@ -1058,8 +1058,7 @@ static DBusMessage *create_proxy(DBusConnection *conn,
> ? ? ? ?if (err == -EINVAL)
> ? ? ? ? ? ? ? ?return __btd_error_invalid_args(msg);
> ? ? ? ?else if (err == -EALREADY)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExist",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Proxy already exists");
> + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> ? ? ? ?else if (err < 0)
> ? ? ? ? ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE "Failed",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Proxy creation failed (%s)", strerror(-err));
> diff --git a/src/adapter.c b/src/adapter.c
> index cc51816..ffbc943 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -1742,9 +1742,7 @@ static DBusMessage *create_device(DBusConnection *conn,
> ? ? ? ? ? ? ? ?return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (adapter_find_device(adapter, address))
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Device already exists");
> + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>
> ? ? ? ?DBG("%s", address);
>
> @@ -1906,9 +1904,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
> ? ? ? ? ? ? ? ?return NULL;
>
> ? ? ? ?if (adapter->agent)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Agent already exists");
> + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
>
> ? ? ? ?cap = parse_io_capability(capability);
> ? ? ? ?if (cap == IO_CAPABILITY_INVALID)
> diff --git a/src/device.c b/src/device.c
> index ef1a910..6d110dc 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -1944,9 +1944,7 @@ DBusMessage *device_create_bonding(struct btd_device *device,
> ? ? ? ?str = textfile_caseget(filename, dstaddr);
> ? ? ? ?if (str) {
> ? ? ? ? ? ? ? ?free(str);
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Bonding already exists");
> + ? ? ? ? ? ? ? return __btd_error_already_exists(msg);
> ? ? ? ?}
>
> ? ? ? ?/* If our IO capability is NoInputNoOutput use medium security
> diff --git a/src/error.c b/src/error.c
> index a30c050..e268163 100644
> --- a/src/error.c
> +++ b/src/error.c
> @@ -55,3 +55,10 @@ DBusMessage *__btd_error_invalid_args(DBusMessage *msg)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?".InvalidArguments",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Invalid arguments in method call");
> ?}
> +
> +DBusMessage *__btd_error_already_exists(DBusMessage *msg)

I also think that an additional message will be great for a better description.

> +{
> + ? ? ? return g_dbus_create_error(msg,
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".AlreadyExists",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Already Exists");
> +}
> diff --git a/src/error.h b/src/error.h
> index 51c094c..284889f 100644
> --- a/src/error.h
> +++ b/src/error.h
> @@ -31,3 +31,4 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char *name, const char *descr);
>
> ?DBusMessage *__btd_error_invalid_args(DBusMessage *msg);
> +DBusMessage *__btd_error_already_exists(DBusMessage *msg);
> --
> 1.7.3.1
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to [email protected]
> More majordomo info at ?http://vger.kernel.org/majordomo-info.html
>

Subject: Re: [PATCH 1/9] Create __btd_error_invalid_args()

Hi Gustavo,

2010/11/8 Gustavo F. Padovan <[email protected]>:
> DBus error handling in BlueZ is a mess. This is the first patch to unify
> all DBus error handling like in ConnMan and oFono. This unifies all
> .InvalidArguments errors.
> ---
> ?attrib/client.c ? ? ? ? ?| ? 20 +++++----------
> ?audio/gateway.c ? ? ? ? ?| ? ?8 +----
> ?audio/headset.c ? ? ? ? ?| ? 18 ++++---------
> ?audio/media.c ? ? ? ? ? ?| ? ?9 ++----
> ?audio/telephony-dummy.c ?| ? 25 ++++++++------------
> ?audio/telephony-maemo5.c | ? 11 ++------
> ?audio/telephony-maemo6.c | ? 11 ++------
> ?audio/transport.c ? ? ? ?| ? 14 +++--------
> ?health/hdp.c ? ? ? ? ? ? | ? 58 ++++++++++++----------------------------------
> ?network/server.c ? ? ? ? | ? ?7 -----
> ?plugins/service.c ? ? ? ?| ? ?8 +-----
> ?serial/port.c ? ? ? ? ? ?| ? ?8 ------
> ?serial/proxy.c ? ? ? ? ? | ? 19 ++++----------
> ?src/adapter.c ? ? ? ? ? ?| ? 52 ++++++++++++++++++-----------------------
> ?src/device.c ? ? ? ? ? ? | ? 22 ++++++-----------
> ?src/error.c ? ? ? ? ? ? ?| ? ?7 +++++
> ?src/error.h ? ? ? ? ? ? ?| ? ?2 +
> ?src/manager.c ? ? ? ? ? ?| ? ?7 -----
> ?18 files changed, 100 insertions(+), 206 deletions(-)
>
> diff --git a/attrib/client.c b/attrib/client.c
> index 1f2c217..cd6e401 100644
> --- a/attrib/client.c
> +++ b/attrib/client.c
> @@ -190,12 +190,6 @@ static int watcher_cmp(gconstpointer a, gconstpointer b)
> ? ? ? ?return g_strcmp0(watcher->path, match->path);
> ?}
>
> -static inline DBusMessage *invalid_args(DBusMessage *msg)
> -{
> - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> -}
> -
> ?static inline DBusMessage *not_authorized(DBusMessage *msg)
> ?{
> ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
> @@ -465,7 +459,7 @@ static DBusMessage *register_watcher(DBusConnection *conn,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (l2cap_connect(prim->gatt, &gerr, TRUE) < 0) {
> ? ? ? ? ? ? ? ?DBusMessage *reply;
> @@ -499,7 +493,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?match = g_new0(struct watcher, 1);
> ? ? ? ?match->name = g_strdup(sender);
> @@ -537,7 +531,7 @@ static DBusMessage *set_value(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY ||
> ? ? ? ? ? ? ? ? ? ? ? ?dbus_message_iter_get_element_type(iter) != DBUS_TYPE_BYTE)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?dbus_message_iter_recurse(iter, &sub);
>
> @@ -586,23 +580,23 @@ static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ?const char *property;
>
> ? ? ? ?if (!dbus_message_iter_init(msg, &iter))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?dbus_message_iter_get_basic(&iter, &property);
> ? ? ? ?dbus_message_iter_next(&iter);
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?dbus_message_iter_recurse(&iter, &sub);
>
> ? ? ? ?if (g_str_equal("Value", property))
> ? ? ? ? ? ? ? ?return set_value(conn, msg, &sub, chr);
>
> - ? ? ? return invalid_args(msg);
> + ? ? ? return __btd_error_invalid_args(msg);
> ?}
>
> ?static GDBusMethodTable char_methods[] = {
> diff --git a/audio/gateway.c b/audio/gateway.c
> index 07ebdd4..ab7d310 100644
> --- a/audio/gateway.c
> +++ b/audio/gateway.c
> @@ -494,9 +494,7 @@ static DBusMessage *register_agent(DBusConnection *conn,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid argument");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?name = dbus_message_get_sender(msg);
> ? ? ? ?agent = g_new0(struct hf_agent, 1);
> @@ -529,9 +527,7 @@ static DBusMessage *unregister_agent(DBusConnection *conn,
> ? ? ? ?if (!dbus_message_get_args(msg, NULL,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_OBJECT_PATH, &path,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid argument");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (strcmp(gw->agent->path, path) != 0)
> ? ? ? ? ? ? ? ?return g_dbus_create_error(msg,
> diff --git a/audio/headset.c b/audio/headset.c
> index 0763585..2cca5ca 100644
> --- a/audio/headset.c
> +++ b/audio/headset.c
> @@ -180,12 +180,6 @@ struct event {
>
> ?static GSList *headset_callbacks = NULL;
>
> -static inline DBusMessage *invalid_args(DBusMessage *msg)
> -{
> - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> -}
> -
> ?static DBusHandlerResult error_not_supported(DBusConnection *conn,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBusMessage *msg)
> ?{
> @@ -2026,35 +2020,35 @@ static DBusMessage *hs_set_property(DBusConnection *conn,
> ? ? ? ?uint16_t gain;
>
> ? ? ? ?if (!dbus_message_iter_init(msg, &iter))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?dbus_message_iter_get_basic(&iter, &property);
> ? ? ? ?dbus_message_iter_next(&iter);
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ?dbus_message_iter_recurse(&iter, &sub);
>
> ? ? ? ?if (g_str_equal("SpeakerGain", property)) {
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT16)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &gain);
> ? ? ? ? ? ? ? ?return hs_set_gain(conn, msg, data, gain,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?HEADSET_GAIN_SPEAKER);
> ? ? ? ?} else if (g_str_equal("MicrophoneGain", property)) {
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT16)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &gain);
> ? ? ? ? ? ? ? ?return hs_set_gain(conn, msg, data, gain,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?HEADSET_GAIN_MICROPHONE);
> ? ? ? ?}
>
> - ? ? ? return invalid_args(msg);
> + ? ? ? return __btd_error_invalid_args(msg);
> ?}
> ?static GDBusMethodTable headset_methods[] = {
> ? ? ? ?{ "Connect", ? ? ? ? ? ?"", ? ? "", ? ? hs_connect,
> diff --git a/audio/media.c b/audio/media.c
> index b6c90f9..862cee6 100644
> --- a/audio/media.c
> +++ b/audio/media.c
> @@ -323,18 +323,15 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?dbus_message_iter_recurse(&args, &props);
> ? ? ? ?if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ".Failed", "Invalid argument");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (parse_properties(&props, &uuid, &delay_reporting, &codec,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&capabilities, &size) || uuid == NULL)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid argument");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (media_endpoint_create(adapter, sender, path, uuid, delay_reporting,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?codec, capabilities, size) == FALSE)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid argument");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
> ?}
> diff --git a/audio/telephony-dummy.c b/audio/telephony-dummy.c
> index 06cb798..b56b6e7 100644
> --- a/audio/telephony-dummy.c
> +++ b/audio/telephony-dummy.c
> @@ -35,6 +35,7 @@
>
> ?#include "log.h"
> ?#include "telephony.h"
> +#include "error.h"
>
> ?#define TELEPHONY_DUMMY_IFACE "org.bluez.TelephonyTest"
> ?#define TELEPHONY_DUMMY_PATH "/org/bluez/test"
> @@ -69,12 +70,6 @@ static struct indicator dummy_indicators[] =
> ? ? ? ?{ NULL }
> ?};
>
> -static inline DBusMessage *invalid_args(DBusMessage *msg)
> -{
> - ? ? ? return g_dbus_create_error(msg, "org.bluez.Error.InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> -}
> -
> ?void telephony_device_connected(void *telephony_device)
> ?{
> ? ? ? ?DBG("telephony-dummy: device %p connected", telephony_device);
> @@ -236,7 +231,7 @@ static DBusMessage *outgoing_call(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?DBG("telephony-dummy: outgoing call to %s", number);
>
> @@ -261,7 +256,7 @@ static DBusMessage *incoming_call(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?DBG("telephony-dummy: incoming call to %s", number);
>
> @@ -307,10 +302,10 @@ static DBusMessage *signal_strength(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UINT32, &strength,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (strength > 5)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?telephony_update_indicator(dummy_indicators, "signal", strength);
>
> @@ -326,10 +321,10 @@ static DBusMessage *battery_level(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UINT32, &level,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (level > 5)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?telephony_update_indicator(dummy_indicators, "battchg", level);
>
> @@ -346,7 +341,7 @@ static DBusMessage *roaming_status(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_BOOLEAN, &roaming,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?val = roaming ? EV_ROAM_ACTIVE : EV_ROAM_INACTIVE;
>
> @@ -365,7 +360,7 @@ static DBusMessage *registration_status(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_BOOLEAN, &registration,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?val = registration ? EV_SERVICE_PRESENT : EV_SERVICE_NONE;
>
> @@ -384,7 +379,7 @@ static DBusMessage *set_subscriber_number(DBusConnection *conn,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?g_free(subscriber_number);
> ? ? ? ?subscriber_number = g_strdup(number);
> diff --git a/audio/telephony-maemo5.c b/audio/telephony-maemo5.c
> index 4d0134c..0f9819c 100644
> --- a/audio/telephony-maemo5.c
> +++ b/audio/telephony-maemo5.c
> @@ -38,6 +38,7 @@
>
> ?#include "log.h"
> ?#include "telephony.h"
> +#include "error.h"
>
> ?/* SSC D-Bus definitions */
> ?#define SSC_DBUS_NAME ?"com.nokia.phone.SSC"
> @@ -1880,12 +1881,6 @@ static void csd_init(void)
> ? ? ? ?}
> ?}
>
> -static inline DBusMessage *invalid_args(DBusMessage *msg)
> -{
> - ? ? ? return g_dbus_create_error(msg,"org.bluez.Error.InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> -}
> -
> ?static uint32_t get_callflag(const char *callerid_setting)
> ?{
> ? ? ? ?if (callerid_setting != NULL) {
> @@ -1950,7 +1945,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
> ? ? ? ?if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&callerid_setting,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID) == FALSE)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (g_str_equal(callerid_setting, "allowed") ||
> ? ? ? ? ? ? ? ? ? ? ? ?g_str_equal(callerid_setting, "restricted") ||
> @@ -1964,7 +1959,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?error("telephony-maemo: invalid argument %s for method call"
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?" SetCallerId", callerid_setting);
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ?}
>
> ?static GDBusMethodTable telephony_maemo_methods[] = {
> diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
> index 72c8e36..4663b4d 100644
> --- a/audio/telephony-maemo6.c
> +++ b/audio/telephony-maemo6.c
> @@ -38,6 +38,7 @@
>
> ?#include "log.h"
> ?#include "telephony.h"
> +#include "error.h"
>
> ?/* SSC D-Bus definitions */
> ?#define SSC_DBUS_NAME ?"com.nokia.phone.SSC"
> @@ -1760,12 +1761,6 @@ static void csd_init(void)
> ? ? ? ?}
> ?}
>
> -static inline DBusMessage *invalid_args(DBusMessage *msg)
> -{
> - ? ? ? return g_dbus_create_error(msg,"org.bluez.Error.InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> -}
> -
> ?static uint32_t get_callflag(const char *callerid_setting)
> ?{
> ? ? ? ?if (callerid_setting != NULL) {
> @@ -1830,7 +1825,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
> ? ? ? ?if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?&callerid_setting,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID) == FALSE)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (g_str_equal(callerid_setting, "allowed") ||
> ? ? ? ? ? ? ? ? ? ? ? ?g_str_equal(callerid_setting, "restricted") ||
> @@ -1844,7 +1839,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?error("telephony-maemo6: invalid argument %s for method call"
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?" SetCallerId", callerid_setting);
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ?}
>
> ?static DBusMessage *clear_lastnumber(DBusConnection *conn, DBusMessage *msg,
> diff --git a/audio/transport.c b/audio/transport.c
> index eda46e1..0c865f7 100644
> --- a/audio/transport.c
> +++ b/audio/transport.c
> @@ -93,12 +93,6 @@ struct media_transport {
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBusMessageIter *value);
> ?};
>
> -static inline DBusMessage *invalid_args(DBusMessage *msg)
> -{
> - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> -}
> -
> ?static inline DBusMessage *error_failed(DBusMessage *msg, const char *desc)
> ?{
> ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "%s", desc);
> @@ -549,16 +543,16 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
> ? ? ? ?int err;
>
> ? ? ? ?if (!dbus_message_iter_init(msg, &iter))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?dbus_message_iter_get_basic(&iter, &property);
> ? ? ? ?dbus_message_iter_next(&iter);
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ?dbus_message_iter_recurse(&iter, &value);
>
> ? ? ? ?sender = dbus_message_get_sender(msg);
> @@ -577,7 +571,7 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (err < 0) {
> ? ? ? ? ? ? ? ?if (err == -EINVAL)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ? ? ? ? ?return error_failed(msg, strerror(-err));
> ? ? ? ?}
>
> diff --git a/health/hdp.c b/health/hdp.c
> index 1eba8e1..6c1fd98 100644
> --- a/health/hdp.c
> +++ b/health/hdp.c
> @@ -321,15 +321,8 @@ static DBusMessage *manager_create_application(DBusConnection *conn,
>
> ? ? ? ?dbus_message_iter_init(msg, &iter);
> ? ? ? ?app = hdp_get_app_config(&iter, &err);
> - ? ? ? if (err) {
> - ? ? ? ? ? ? ? DBusMessage *reply;
> -
> - ? ? ? ? ? ? ? reply = g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments: %s", err->message);
> - ? ? ? ? ? ? ? g_error_free(err);
> - ? ? ? ? ? ? ? return reply;
> - ? ? ? }
> + ? ? ? if (err)
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);


You are leaking memory here, err should be freed before return. Also
the message that we add to the error is for clarify the user the kind
of error, it will be great to keep it.

>
> ? ? ? ?name = dbus_message_get_sender(msg);
> ? ? ? ?if (!name) {
> @@ -368,11 +361,8 @@ static DBusMessage *manager_destroy_application(DBusConnection *conn,
> ? ? ? ?GSList *l;
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DBUS_TYPE_INVALID)){
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> - ? ? ? }
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DBUS_TYPE_INVALID))
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?l = g_slist_find_custom(applications, path, cmp_app);
>
> @@ -1801,18 +1791,13 @@ static DBusMessage *device_create_channel(DBusConnection *conn,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &app_path,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_STRING, &conf,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DBUS_TYPE_INVALID)) {
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> - ? ? ? }
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? DBUS_TYPE_INVALID))
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?l = g_slist_find_custom(applications, app_path, cmp_app);
> ? ? ? ?if (!l)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call, "
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "no such application");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> +
> ? ? ? ?app = l->data;
>
> ? ? ? ?if (g_ascii_strcasecmp("Reliable", conf) == 0)
> @@ -1822,25 +1807,16 @@ static DBusMessage *device_create_channel(DBusConnection *conn,
> ? ? ? ?else if (g_ascii_strcasecmp("Any", conf) == 0)
> ? ? ? ? ? ? ? ?config = HDP_NO_PREFERENCE_DC;
> ? ? ? ?else
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (app->role == HDP_SINK && config != HDP_NO_PREFERENCE_DC)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Configuration not valid for sinks");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);

Also here the message we tried to clarify the kind of "Invalid
Arguments" error with a different message.

>
> ? ? ? ?if (app->role == HDP_SOURCE && config == HDP_NO_PREFERENCE_DC)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Configuration not valid for sources");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);

And here.

>
> ? ? ? ?if (!device->fr && config == HDP_STREAMING_DC)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Configuration not valid, first "
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "channel should be reliable");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);


And here.

>
> ? ? ? ?data = g_new0(struct hdp_create_dc, 1);
> ? ? ? ?data->dev = device;
> @@ -1924,17 +1900,13 @@ static DBusMessage *device_destroy_channel(DBusConnection *conn,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID)){
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ?}
>
> ? ? ? ?l = g_slist_find_custom(device->channels, path, cmp_chan_path);
> ? ? ? ?if (!l)
> - ? ? ? ? ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call, "
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "no such channel");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> +


And here.

> ? ? ? ?hdp_chan = l->data;
> ? ? ? ?del_data = g_new0(struct hdp_tmp_dc_data, 1);
> ? ? ? ?del_data->msg = dbus_message_ref(msg);
> diff --git a/network/server.c b/network/server.c
> index e6706ec..ce1fe5e 100644
> --- a/network/server.c
> +++ b/network/server.c
> @@ -575,13 +575,6 @@ static inline DBusMessage *failed(DBusMessage *msg, const char *description)
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"%s", description);
> ?}
>
> -static inline DBusMessage *invalid_arguments(DBusMessage *msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const char *description)
> -{
> - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "%s", description);
> -}
> -
> ?static void server_disconnect(DBusConnection *conn, void *user_data)
> ?{
> ? ? ? ?struct network_server *ns = user_data;
> diff --git a/plugins/service.c b/plugins/service.c
> index f6b9da7..6f6a275 100644
> --- a/plugins/service.c
> +++ b/plugins/service.c
> @@ -339,12 +339,6 @@ static void exit_callback(DBusConnection *conn, void *user_data)
> ? ? ? ?g_free(user_record);
> ?}
>
> -static inline DBusMessage *invalid_arguments(DBusMessage *msg)
> -{
> - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> -}
> -
> ?static inline DBusMessage *not_available(DBusMessage *msg)
> ?{
> ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
> @@ -466,7 +460,7 @@ static DBusMessage *update_xml_record(DBusConnection *conn,
>
> ? ? ? ?len = (record ? strlen(record) : 0);
> ? ? ? ?if (len == 0)
> - ? ? ? ? ? ? ? return invalid_arguments(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?user_record = find_record(serv_adapter, handle,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?dbus_message_get_sender(msg));
> diff --git a/serial/port.c b/serial/port.c
> index 0398f2e..b593311 100644
> --- a/serial/port.c
> +++ b/serial/port.c
> @@ -57,7 +57,6 @@
> ?#include "port.h"
>
> ?#define SERIAL_PORT_INTERFACE ?"org.bluez.Serial"
> -#define ERROR_INVALID_ARGS ? ? "org.bluez.Error.InvalidArguments"
> ?#define ERROR_DOES_NOT_EXIST ? "org.bluez.Error.DoesNotExist"
>
> ?#define MAX_OPEN_TRIES ? ? ? ? 5
> @@ -243,13 +242,6 @@ static inline DBusMessage *does_not_exist(DBusMessage *msg,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"%s", description);
> ?}
>
> -static inline DBusMessage *invalid_arguments(DBusMessage *msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const char *description)
> -{
> - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "%s", description);
> -}
> -
> ?static inline DBusMessage *failed(DBusMessage *msg, const char *description)
> ?{
> ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
> diff --git a/serial/proxy.c b/serial/proxy.c
> index 3aaabf3..8e182b6 100644
> --- a/serial/proxy.c
> +++ b/serial/proxy.c
> @@ -140,13 +140,6 @@ static inline DBusMessage *does_not_exist(DBusMessage *msg,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"%s", description);
> ?}
>
> -static inline DBusMessage *invalid_arguments(DBusMessage *msg,
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? const char *description)
> -{
> - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "%s", description);
> -}
> -
> ?static inline DBusMessage *failed(DBusMessage *msg, const char *description)
> ?{
> ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
> @@ -755,17 +748,17 @@ static DBusMessage *proxy_set_serial_params(DBusConnection *conn,
> ? ? ? ? ? ? ? ?return NULL;
>
> ? ? ? ?if (str2speed(ratestr, &speed) ?== B0)
> - ? ? ? ? ? ? ? return invalid_arguments(msg, "Invalid baud rate");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);

Also here there is a clarification about the invalid arguments with a
better error description, and all others in this file.

>
> ? ? ? ?ctrl = prx->proxy_ti.c_cflag;
> ? ? ? ?if (set_databits(databits, &ctrl) < 0)
> - ? ? ? ? ? ? ? return invalid_arguments(msg, "Invalid data bits");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (set_stopbits(stopbits, &ctrl) < 0)
> - ? ? ? ? ? ? ? return invalid_arguments(msg, "Invalid stop bits");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (set_parity(paritystr, &ctrl) < 0)
> - ? ? ? ? ? ? ? return invalid_arguments(msg, "Invalid parity");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?prx->proxy_ti.c_cflag = ctrl;
> ? ? ? ?prx->proxy_ti.c_cflag |= (CLOCAL | CREAD);
> @@ -1057,13 +1050,13 @@ static DBusMessage *create_proxy(DBusConnection *conn,
>
> ? ? ? ?uuid_str = bt_name2string(pattern);
> ? ? ? ?if (!uuid_str)
> - ? ? ? ? ? ? ? return invalid_arguments(msg, "Invalid UUID");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?err = register_proxy(adapter, uuid_str, address, &proxy);
> ? ? ? ?g_free(uuid_str);
>
> ? ? ? ?if (err == -EINVAL)
> - ? ? ? ? ? ? ? return invalid_arguments(msg, "Invalid address");
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ?else if (err == -EALREADY)
> ? ? ? ? ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExist",
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?"Proxy already exists");
> diff --git a/src/adapter.c b/src/adapter.c
> index 31014e5..cc51816 100644
> --- a/src/adapter.c
> +++ b/src/adapter.c
> @@ -148,12 +148,6 @@ struct btd_adapter {
> ?static void adapter_set_pairable_timeout(struct btd_adapter *adapter,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?guint interval);
>
> -static inline DBusMessage *invalid_args(DBusMessage *msg)
> -{
> - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> -}
> -
> ?static inline DBusMessage *adapter_not_ready(DBusMessage *msg)
> ?{
> ? ? ? ?return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady",
> @@ -1088,7 +1082,7 @@ static DBusMessage *set_name(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (!g_utf8_validate(name, -1, NULL)) {
> ? ? ? ? ? ? ? ?error("Name change failed: supplied name isn't valid UTF-8");
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ?}
>
> ? ? ? ?if (strncmp(name, (char *) dev->name, MAX_NAME_LENGTH) == 0)
> @@ -1535,23 +1529,23 @@ static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ?ba2str(&adapter->bdaddr, srcaddr);
>
> ? ? ? ?if (!dbus_message_iter_init(msg, &iter))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?dbus_message_iter_get_basic(&iter, &property);
> ? ? ? ?dbus_message_iter_next(&iter);
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ?dbus_message_iter_recurse(&iter, &sub);
>
> ? ? ? ?if (g_str_equal("Name", property)) {
> ? ? ? ? ? ? ? ?const char *name;
>
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_STRING)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &name);
>
> ? ? ? ? ? ? ? ?return set_name(conn, msg, name, data);
> @@ -1559,7 +1553,7 @@ static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ? ? ? ? ?gboolean powered;
>
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &powered);
>
> @@ -1568,7 +1562,7 @@ static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ? ? ? ? ?gboolean discoverable;
>
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &discoverable);
>
> @@ -1577,7 +1571,7 @@ static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ? ? ? ? ?uint32_t timeout;
>
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT32)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &timeout);
>
> @@ -1586,7 +1580,7 @@ static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ? ? ? ? ?gboolean pairable;
>
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &pairable);
>
> @@ -1595,14 +1589,14 @@ static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ? ? ? ? ?uint32_t timeout;
>
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT32)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &timeout);
>
> ? ? ? ? ? ? ? ?return set_pairable_timeout(conn, msg, timeout, data);
> ? ? ? ?}
>
> - ? ? ? return invalid_args(msg);
> + ? ? ? return __btd_error_invalid_args(msg);
> ?}
>
> ?static DBusMessage *request_session(DBusConnection *conn,
> @@ -1676,7 +1670,7 @@ static DBusMessage *list_devices(DBusConnection *conn,
> ? ? ? ?const gchar *dev_path;
>
> ? ? ? ?if (!dbus_message_has_signature(msg, DBUS_TYPE_INVALID_AS_STRING))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?reply = dbus_message_new_method_return(msg);
> ? ? ? ?if (!reply)
> @@ -1709,10 +1703,10 @@ static DBusMessage *cancel_device_creation(DBusConnection *conn,
>
> ? ? ? ?if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID) == FALSE)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (check_address(address) < 0)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?device = adapter_find_device(adapter, address);
> ? ? ? ?if (!device || !device_is_creating(device, NULL))
> @@ -1742,10 +1736,10 @@ static DBusMessage *create_device(DBusConnection *conn,
>
> ? ? ? ?if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID) == FALSE)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (check_address(address) < 0)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (adapter_find_device(adapter, address))
> ? ? ? ? ? ? ? ?return g_dbus_create_error(msg,
> @@ -1790,21 +1784,21 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_OBJECT_PATH, &agent_path,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_STRING, &capability,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID) == FALSE)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (check_address(address) < 0)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?sender = dbus_message_get_sender(msg);
> ? ? ? ?if (adapter->agent &&
> ? ? ? ? ? ? ? ? ? ? ? ?agent_matches(adapter->agent, sender, agent_path)) {
> ? ? ? ? ? ? ? ?error("Refusing adapter agent usage as device specific one");
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ?}
>
> ? ? ? ?cap = parse_io_capability(capability);
> ? ? ? ?if (cap == IO_CAPABILITY_INVALID)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?device = adapter_get_device(conn, adapter, address);
> ? ? ? ?if (!device)
> @@ -1832,7 +1826,7 @@ static DBusMessage *remove_device(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID) == FALSE)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?l = g_slist_find_custom(adapter->devices,
> ? ? ? ? ? ? ? ? ? ? ? ?path, (GCompareFunc) device_path_cmp);
> @@ -1870,7 +1864,7 @@ static DBusMessage *find_device(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBUS_TYPE_INVALID))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?l = g_slist_find_custom(adapter->devices,
> ? ? ? ? ? ? ? ? ? ? ? ?address, (GCompareFunc) device_address_cmp);
> @@ -1918,7 +1912,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?cap = parse_io_capability(capability);
> ? ? ? ?if (cap == IO_CAPABILITY_INVALID)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?name = dbus_message_get_sender(msg);
>
> diff --git a/src/device.c b/src/device.c
> index c2b6682..ef1a910 100644
> --- a/src/device.c
> +++ b/src/device.c
> @@ -549,12 +549,6 @@ static DBusMessage *set_blocked(DBusConnection *conn, DBusMessage *msg,
> ? ? ? ?}
> ?}
>
> -static inline DBusMessage *invalid_args(DBusMessage *msg)
> -{
> - ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> -}
> -
> ?static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?DBusMessage *msg, void *data)
> ?{
> @@ -563,22 +557,22 @@ static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ?const char *property;
>
> ? ? ? ?if (!dbus_message_iter_init(msg, &iter))
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ?dbus_message_iter_get_basic(&iter, &property);
> ? ? ? ?dbus_message_iter_next(&iter);
>
> ? ? ? ?if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
> - ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ?dbus_message_iter_recurse(&iter, &sub);
>
> ? ? ? ?if (g_str_equal("Trusted", property)) {
> ? ? ? ? ? ? ? ?dbus_bool_t value;
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &value);
>
> ? ? ? ? ? ? ? ?return set_trust(conn, msg, value, data);
> @@ -586,7 +580,7 @@ static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ? ? ? ? ?const char *alias;
>
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_STRING)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &alias);
>
> ? ? ? ? ? ? ? ?return set_alias(conn, msg, alias, data);
> @@ -594,14 +588,14 @@ static DBusMessage *set_property(DBusConnection *conn,
> ? ? ? ? ? ? ? ?dbus_bool_t value;
>
> ? ? ? ? ? ? ? ?if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ? ? ? ? ?dbus_message_iter_get_basic(&sub, &value);
>
> ? ? ? ? ? ? ? ?return set_blocked(conn, msg, value, data);
> ? ? ? ?}
>
> - ? ? ? return invalid_args(msg);
> + ? ? ? return __btd_error_invalid_args(msg);
> ?}
>
> ?static void discover_services_req_exit(DBusConnection *conn, void *user_data)
> @@ -636,7 +630,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
> ? ? ? ? ? ? ? ?uuid_t uuid;
>
> ? ? ? ? ? ? ? ?if (bt_string2uuid(&uuid, pattern) < 0)
> - ? ? ? ? ? ? ? ? ? ? ? return invalid_args(msg);
> + ? ? ? ? ? ? ? ? ? ? ? return __btd_error_invalid_args(msg);
>
> ? ? ? ? ? ? ? ?sdp_uuid128_to_uuid(&uuid);
>
> diff --git a/src/error.c b/src/error.c
> index c1a2fbf..a30c050 100644
> --- a/src/error.c
> +++ b/src/error.c
> @@ -48,3 +48,10 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
>
> ? ? ? ?return DBUS_HANDLER_RESULT_HANDLED;
> ?}
> +
> +DBusMessage *__btd_error_invalid_args(DBusMessage *msg)

I think that it will be great to add an optional parameter for a
better description of the error in the case it is required.

> +{
> + ? ? ? return g_dbus_create_error(msg, ERROR_INTERFACE
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ".InvalidArguments",
> + ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> +}
> diff --git a/src/error.h b/src/error.h
> index 49ec05e..51c094c 100644
> --- a/src/error.h
> +++ b/src/error.h
> @@ -29,3 +29,5 @@
>
> ?DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
> ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ? ?const char *name, const char *descr);
> +
> +DBusMessage *__btd_error_invalid_args(DBusMessage *msg);
> diff --git a/src/manager.c b/src/manager.c
> index 3fad9e6..315f5c3 100644
> --- a/src/manager.c
> +++ b/src/manager.c
> @@ -66,13 +66,6 @@ void manager_update_svc(struct btd_adapter* adapter, uint8_t svc)
> ? ? ? ?adapter_set_service_classes(adapter, svc);
> ?}
>
> -static inline DBusMessage *invalid_args(DBusMessage *msg)
> -{
> - ? ? ? return g_dbus_create_error(msg,
> - ? ? ? ? ? ? ? ? ? ? ? ERROR_INTERFACE ".InvalidArguments",
> - ? ? ? ? ? ? ? ? ? ? ? "Invalid arguments in method call");
> -}
> -
> ?static inline DBusMessage *no_such_adapter(DBusMessage *msg)
> ?{
> ? ? ? ?return g_dbus_create_error(msg,
> --
> 1.7.3.1
>

2010-11-08 17:31:33

by Gustavo Padovan

[permalink] [raw]
Subject: Re: [PATCH 0/9] Fixing DBus error system in BlueZ

Hi Johan,

* Johan Hedberg <[email protected]> [2010-11-08 14:33:30 +0200]:

> Hi Gustavo,
>
> On Mon, Nov 08, 2010, Gustavo F. Padovan wrote:
> > Here are some patches that try to fix the mess of reporting error to
> > DBus inside BlueZ. It follows the oFono and ConnMan error system.
> >
> > The goal is to get ride of any directly call to g_dbus_create_error()
> > inside bluez code, changing that to __btd_error_*. This patch set
> > doesn't fix all of them yet, but is a very good start. Please review.
> >
> >
> > Gustavo F. Padovan (9):
> > Create __btd_error_invalid_args()
> > Add __btd_error_already_exists()
> > Add __btd_error_not_supported()
> > Add __btd_error_not_connected()
> > Add __btd_error_in_progress()
> > Add __btd_error_not_available()
> > Add __btd_error_busy()
> > Add __btd_error_does_not_exist()
> > Add __btd_error_not_authorized()
>
> The patches seem fine to me, but before pushing upstream I'd like to
> understand the reason for prefixing these with with __btd instead of
> btd. What's the criteria used to decide what to use and when and why is
> __btd the correct choice for these new functions? My first guess would
> have been that __btd is for things only accessible by the core-daemon
> whereas btd is for functions exported to plugins, but that doesn't seem
> to be the case with your patches since many of these __btd functions get
> called from plugins.

I just followed oFono and ConnMan on this. That is the reason and I
didn't asked myself why have a __ in this case.. But I see your point.
Do you think that change that to btd_error_* will fit better inside
BlueZ? I can change that then.

--
Gustavo F. Padovan
ProFUSION embedded systems - http://profusion.mobi

2010-11-08 12:33:30

by Johan Hedberg

[permalink] [raw]
Subject: Re: [PATCH 0/9] Fixing DBus error system in BlueZ

Hi Gustavo,

On Mon, Nov 08, 2010, Gustavo F. Padovan wrote:
> Here are some patches that try to fix the mess of reporting error to
> DBus inside BlueZ. It follows the oFono and ConnMan error system.
>
> The goal is to get ride of any directly call to g_dbus_create_error()
> inside bluez code, changing that to __btd_error_*. This patch set
> doesn't fix all of them yet, but is a very good start. Please review.
>
>
> Gustavo F. Padovan (9):
> Create __btd_error_invalid_args()
> Add __btd_error_already_exists()
> Add __btd_error_not_supported()
> Add __btd_error_not_connected()
> Add __btd_error_in_progress()
> Add __btd_error_not_available()
> Add __btd_error_busy()
> Add __btd_error_does_not_exist()
> Add __btd_error_not_authorized()

The patches seem fine to me, but before pushing upstream I'd like to
understand the reason for prefixing these with with __btd instead of
btd. What's the criteria used to decide what to use and when and why is
__btd the correct choice for these new functions? My first guess would
have been that __btd is for things only accessible by the core-daemon
whereas btd is for functions exported to plugins, but that doesn't seem
to be the case with your patches since many of these __btd functions get
called from plugins.

Johan

2010-11-08 06:29:47

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 9/9] Add __btd_error_not_authorized()

---
attrib/client.c | 8 +-------
plugins/service.c | 22 ++++++++--------------
src/adapter.c | 8 +-------
src/device.c | 8 ++------
src/error.c | 7 +++++++
src/error.h | 1 +
6 files changed, 20 insertions(+), 34 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index cd6e401..aadb28c 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -190,12 +190,6 @@ static int watcher_cmp(gconstpointer a, gconstpointer b)
return g_strcmp0(watcher->path, match->path);
}

-static inline DBusMessage *not_authorized(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
- "Not authorized");
-}
-
static void append_char_dict(DBusMessageIter *iter, struct characteristic *chr)
{
DBusMessageIter dict;
@@ -501,7 +495,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn,
l = g_slist_find_custom(prim->watchers, match, watcher_cmp);
watcher_free(match);
if (!l)
- return not_authorized(msg);
+ return __btd_error_not_authorized(msg);

watcher = l->data;
g_dbus_remove_watch(conn, watcher->id);
diff --git a/plugins/service.c b/plugins/service.c
index b9f4b97..4e815fb 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -350,12 +350,6 @@ static inline DBusMessage *failed_strerror(DBusMessage *msg, int err)
"%s", strerror(err));
}

-static inline DBusMessage *not_authorized(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
- "Not Authorized");
-}
-
static int add_xml_record(DBusConnection *conn, const char *sender,
struct service_adapter *serv_adapter,
const char *record, dbus_uint32_t *handle)
@@ -557,7 +551,7 @@ static void auth_cb(DBusError *derr, void *user_data)
if (derr) {
error("Access denied: %s", derr->message);

- reply = not_authorized(auth->msg);
+ reply = __btd_error_not_authorized(auth->msg);
dbus_message_unref(auth->msg);
g_dbus_send_message(auth->conn, reply);
goto done;
@@ -614,20 +608,20 @@ static DBusMessage *request_authorization(DBusConnection *conn,
if (!user_record) {
user_record = find_record(serv_adapter_any, handle, sender);
if (!user_record)
- return not_authorized(msg);
+ return __btd_error_not_authorized(msg);
}

record = sdp_record_find(user_record->handle);
if (record == NULL)
- return not_authorized(msg);
+ return __btd_error_not_authorized(msg);

if (sdp_get_service_classes(record, &services) < 0) {
sdp_record_free(record);
- return not_authorized(msg);
+ return __btd_error_not_authorized(msg);
}

if (services == NULL)
- return not_authorized(msg);
+ return __btd_error_not_authorized(msg);

uuid = services->data;
uuid128 = sdp_uuid_to_uuid128(uuid);
@@ -636,7 +630,7 @@ static DBusMessage *request_authorization(DBusConnection *conn,

if (sdp_uuid2strn(uuid128, uuid_str, MAX_LEN_UUID_STR) < 0) {
bt_free(uuid128);
- return not_authorized(msg);
+ return __btd_error_not_authorized(msg);
}
bt_free(uuid128);

@@ -664,7 +658,7 @@ static DBusMessage *request_authorization(DBusConnection *conn,
serv_adapter->pending_list = g_slist_remove(serv_adapter->pending_list,
auth);
g_free(auth);
- return not_authorized(msg);
+ return __btd_error_not_authorized(msg);
}

return NULL;
@@ -692,7 +686,7 @@ static DBusMessage *cancel_authorization(DBusConnection *conn,

btd_cancel_authorization(&src, &auth->dst);

- reply = not_authorized(auth->msg);
+ reply = __btd_error_not_authorized(auth->msg);
dbus_message_unref(auth->msg);
g_dbus_send_message(auth->conn, reply);

diff --git a/src/adapter.c b/src/adapter.c
index 68ae6e4..9ca41cb 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -166,12 +166,6 @@ static inline DBusMessage *not_in_progress(DBusMessage *msg, const char *str)
"%s", str);
}

-static inline DBusMessage *not_authorized(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
- "Not authorized");
-}
-
static int found_device_cmp(const struct remote_dev_info *d1,
const struct remote_dev_info *d2)
{
@@ -1713,7 +1707,7 @@ static DBusMessage *cancel_device_creation(DBusConnection *conn,
return not_in_progress(msg, "Device creation not in progress");

if (!device_is_creating(device, sender))
- return not_authorized(msg);
+ return __btd_error_not_authorized(msg);

device_set_temporary(device, TRUE);

diff --git a/src/device.c b/src/device.c
index f72a9b5..5d5f23d 100644
--- a/src/device.c
+++ b/src/device.c
@@ -729,17 +729,13 @@ static DBusMessage *cancel_discover(DBusConnection *conn,

if (!dbus_message_is_method_call(device->browse->msg, DEVICE_INTERFACE,
"DiscoverServices"))
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".NotAuthorized",
- "Not Authorized");
+ return __btd_error_not_authorized(msg);

requestor = browse_request_get_requestor(device->browse);

/* only the discover requestor can cancel the inquiry process */
if (!requestor || !g_str_equal(requestor, sender))
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".NotAuthorized",
- "Not Authorized");
+ return __btd_error_not_authorized(msg);

discover_services_reply(device->browse, -ECANCELED, NULL);

diff --git a/src/error.c b/src/error.c
index c8e5ed7..c382f58 100644
--- a/src/error.c
+++ b/src/error.c
@@ -103,3 +103,10 @@ DBusMessage *__btd_error_does_not_exist(DBusMessage *msg)
".DoesNotExist",
"Does Not Exist");
}
+
+DBusMessage *__btd_error_not_authorized(DBusMessage *msg)
+{
+ return g_dbus_create_error(msg, ERROR_INTERFACE
+ ".NotAuthorized",
+ "Operation Not Authorized");
+}
diff --git a/src/error.h b/src/error.h
index bd8d724..a970c92 100644
--- a/src/error.h
+++ b/src/error.h
@@ -38,3 +38,4 @@ DBusMessage *__btd_error_not_connected(DBusMessage *msg);
DBusMessage *__btd_error_not_available(DBusMessage *msg);
DBusMessage *__btd_error_in_progress(DBusMessage *msg);
DBusMessage *__btd_error_does_not_exist(DBusMessage *msg);
+DBusMessage *__btd_error_not_authorized(DBusMessage *msg);
--
1.7.3.1


2010-11-08 06:29:46

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 8/9] Add __btd_error_does_not_exist()

---
plugins/service.c | 10 ++--------
serial/port.c | 12 ++----------
serial/proxy.c | 9 +--------
src/adapter.c | 13 ++++---------
src/error.c | 7 +++++++
src/error.h | 1 +
6 files changed, 17 insertions(+), 35 deletions(-)

diff --git a/plugins/service.c b/plugins/service.c
index f4b71ae..b9f4b97 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -356,12 +356,6 @@ static inline DBusMessage *not_authorized(DBusMessage *msg)
"Not Authorized");
}

-static inline DBusMessage *does_not_exist(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
- "Does Not Exist");
-}
-
static int add_xml_record(DBusConnection *conn, const char *sender,
struct service_adapter *serv_adapter,
const char *record, dbus_uint32_t *handle)
@@ -658,7 +652,7 @@ static DBusMessage *request_authorization(DBusConnection *conn,

auth = next_pending(serv_adapter);
if (auth == NULL)
- return does_not_exist(msg);
+ return __btd_error_does_not_exist(msg);

if (serv_adapter->adapter)
adapter_get_address(serv_adapter->adapter, &src);
@@ -689,7 +683,7 @@ static DBusMessage *cancel_authorization(DBusConnection *conn,

auth = find_pending_by_sender(serv_adapter, sender);
if (auth == NULL)
- return does_not_exist(msg);
+ return __btd_error_does_not_exist(msg);

if (serv_adapter->adapter)
adapter_get_address(serv_adapter->adapter, &src);
diff --git a/serial/port.c b/serial/port.c
index b593311..c79243e 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -57,7 +57,6 @@
#include "port.h"

#define SERIAL_PORT_INTERFACE "org.bluez.Serial"
-#define ERROR_DOES_NOT_EXIST "org.bluez.Error.DoesNotExist"

#define MAX_OPEN_TRIES 5
#define OPEN_WAIT 300 /* ms. udev node creation retry wait */
@@ -235,13 +234,6 @@ void port_release_all(void)
g_slist_free(devices);
}

-static inline DBusMessage *does_not_exist(DBusMessage *msg,
- const char *description)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
- "%s", description);
-}
-
static inline DBusMessage *failed(DBusMessage *msg, const char *description)
{
return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
@@ -497,7 +489,7 @@ static DBusMessage *port_connect(DBusConnection *conn,

channel = strtol(pattern, &endptr, 10);
if ((endptr && *endptr != '\0') || channel < 1 || channel > 30)
- return does_not_exist(msg, "Does not match");
+ return __btd_error_does_not_exist(msg);

port = create_port(device, NULL, channel);
}
@@ -538,7 +530,7 @@ static DBusMessage *port_disconnect(DBusConnection *conn,

port = find_port(device->ports, dev);
if (!port)
- return does_not_exist(msg, "Port does not exist");
+ return __btd_error_does_not_exist(msg);

if (!port->listener_id)
return failed(msg, "Not connected");
diff --git a/serial/proxy.c b/serial/proxy.c
index de82f9a..9321bd4 100644
--- a/serial/proxy.c
+++ b/serial/proxy.c
@@ -133,13 +133,6 @@ static void proxy_free(struct serial_proxy *prx)
g_free(prx);
}

-static inline DBusMessage *does_not_exist(DBusMessage *msg,
- const char *description)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".DoesNotExist",
- "%s", description);
-}
-
static inline DBusMessage *failed(DBusMessage *msg, const char *description)
{
return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
@@ -1115,7 +1108,7 @@ static DBusMessage *remove_proxy(DBusConnection *conn,

l = g_slist_find_custom(adapter->proxies, path, proxy_pathcmp);
if (!l)
- return does_not_exist(msg, "Invalid proxy path");
+ return __btd_error_does_not_exist(msg);

prx = l->data;

diff --git a/src/adapter.c b/src/adapter.c
index ffbc943..68ae6e4 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1829,9 +1829,8 @@ static DBusMessage *remove_device(DBusConnection *conn, DBusMessage *msg,
l = g_slist_find_custom(adapter->devices,
path, (GCompareFunc) device_path_cmp);
if (!l)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".DoesNotExist",
- "Device does not exist");
+ return __btd_error_does_not_exist(msg);
+
device = l->data;

if (device_is_temporary(device) || device_is_busy(device))
@@ -1867,9 +1866,7 @@ static DBusMessage *find_device(DBusConnection *conn, DBusMessage *msg,
l = g_slist_find_custom(adapter->devices,
address, (GCompareFunc) device_address_cmp);
if (!l)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".DoesNotExist",
- "Device does not exist");
+ return __btd_error_does_not_exist(msg);

device = l->data;

@@ -1940,9 +1937,7 @@ static DBusMessage *unregister_agent(DBusConnection *conn, DBusMessage *msg,
name = dbus_message_get_sender(msg);

if (!adapter->agent || !agent_matches(adapter->agent, name, path))
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".DoesNotExist",
- "No such agent");
+ return __btd_error_does_not_exist(msg);

agent_free(adapter->agent);
adapter->agent = NULL;
diff --git a/src/error.c b/src/error.c
index 8000467..c8e5ed7 100644
--- a/src/error.c
+++ b/src/error.c
@@ -96,3 +96,10 @@ DBusMessage *__btd_error_not_available(DBusMessage *msg)
".NotAvailable",
"Operation currently not available");
}
+
+DBusMessage *__btd_error_does_not_exist(DBusMessage *msg)
+{
+ return g_dbus_create_error(msg, ERROR_INTERFACE
+ ".DoesNotExist",
+ "Does Not Exist");
+}
diff --git a/src/error.h b/src/error.h
index c023809..bd8d724 100644
--- a/src/error.h
+++ b/src/error.h
@@ -37,3 +37,4 @@ DBusMessage *__btd_error_not_supported(DBusMessage *msg);
DBusMessage *__btd_error_not_connected(DBusMessage *msg);
DBusMessage *__btd_error_not_available(DBusMessage *msg);
DBusMessage *__btd_error_in_progress(DBusMessage *msg);
+DBusMessage *__btd_error_does_not_exist(DBusMessage *msg);
--
1.7.3.1


2010-11-08 06:29:45

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 7/9] Add __btd_error_busy()

---
audio/headset.c | 4 +---
audio/sink.c | 6 ++----
audio/source.c | 3 +--
input/device.c | 6 ------
src/error.c | 6 ++++++
src/error.h | 1 +
6 files changed, 11 insertions(+), 15 deletions(-)

diff --git a/audio/headset.c b/audio/headset.c
index 017f02e..15853a8 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -1808,9 +1808,7 @@ static DBusMessage *hs_play(DBusConnection *conn, DBusMessage *msg,
hs->pending->msg = dbus_message_ref(msg);
return NULL;
}
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".InProgress",
- "Play in Progress");
+ return __btd_error_busy(msg);
case HEADSET_STATE_PLAYING:
return g_dbus_create_error(msg, ERROR_INTERFACE
".AlreadyConnected",
diff --git a/audio/sink.c b/audio/sink.c
index 3c55c74..49c8bfc 100644
--- a/audio/sink.c
+++ b/audio/sink.c
@@ -423,8 +423,7 @@ static DBusMessage *sink_connect(DBusConnection *conn,
"Unable to get a session");

if (sink->connect || sink->disconnect)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
- "%s", strerror(EBUSY));
+ return __btd_error_busy(msg);

if (sink->stream_state >= AVDTP_STATE_OPEN)
return g_dbus_create_error(msg, ERROR_INTERFACE
@@ -459,8 +458,7 @@ static DBusMessage *sink_disconnect(DBusConnection *conn,
return __btd_error_not_connected(msg);

if (sink->connect || sink->disconnect)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
- "%s", strerror(EBUSY));
+ return __btd_error_busy(msg);

if (sink->stream_state < AVDTP_STATE_OPEN) {
DBusMessage *reply = dbus_message_new_method_return(msg);
diff --git a/audio/source.c b/audio/source.c
index 4d47853..6767591 100644
--- a/audio/source.c
+++ b/audio/source.c
@@ -390,8 +390,7 @@ static DBusMessage *source_connect(DBusConnection *conn,
"Unable to get a session");

if (source->connect || source->disconnect)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
- "%s", strerror(EBUSY));
+ return __btd_error_busy(msg);

if (source->stream_state >= AVDTP_STATE_OPEN)
return g_dbus_create_error(msg, ERROR_INTERFACE
diff --git a/input/device.c b/input/device.c
index 1ccb2b0..6968b6c 100644
--- a/input/device.c
+++ b/input/device.c
@@ -325,12 +325,6 @@ static inline DBusMessage *not_supported(DBusMessage *msg)
"Not supported");
}

-static inline DBusMessage *in_progress(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
- "Device connection already in progress");
-}
-
static inline DBusMessage *already_connected(DBusMessage *msg)
{
return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyConnected",
diff --git a/src/error.c b/src/error.c
index 8b91a03..8000467 100644
--- a/src/error.c
+++ b/src/error.c
@@ -56,6 +56,12 @@ DBusMessage *__btd_error_invalid_args(DBusMessage *msg)
"Invalid arguments in method call");
}

+DBusMessage *__btd_error_busy(DBusMessage *msg)
+{
+ return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
+ "Operation already in progress");
+}
+
DBusMessage *__btd_error_already_exists(DBusMessage *msg)
{
return g_dbus_create_error(msg,
diff --git a/src/error.h b/src/error.h
index dfdc933..c023809 100644
--- a/src/error.h
+++ b/src/error.h
@@ -31,6 +31,7 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
const char *name, const char *descr);

DBusMessage *__btd_error_invalid_args(DBusMessage *msg);
+DBusMessage *__btd_error_busy(DBusMessage *msg);
DBusMessage *__btd_error_already_exists(DBusMessage *msg);
DBusMessage *__btd_error_not_supported(DBusMessage *msg);
DBusMessage *__btd_error_not_connected(DBusMessage *msg);
--
1.7.3.1


2010-11-08 06:29:44

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 6/9] Add __btd_error_not_available()

---
audio/headset.c | 9 +++------
plugins/service.c | 16 +++-------------
src/error.c | 7 +++++++
src/error.h | 1 +
4 files changed, 14 insertions(+), 19 deletions(-)

diff --git a/audio/headset.c b/audio/headset.c
index fd358a0..017f02e 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -1796,8 +1796,7 @@ static DBusMessage *hs_play(DBusConnection *conn, DBusMessage *msg,
if (sco_hci) {
error("Refusing Headset.Play() because SCO HCI routing "
"is enabled");
- return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
- "Operation not Available");
+ return __btd_error_not_available(msg);
}

switch (hs->state) {
@@ -1842,8 +1841,7 @@ static DBusMessage *hs_get_speaker_gain(DBusConnection *conn,
dbus_uint16_t gain;

if (hs->state < HEADSET_STATE_CONNECTED)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
- "Operation not Available");
+ return __btd_error_not_available(msg);

reply = dbus_message_new_method_return(msg);
if (!reply)
@@ -1868,8 +1866,7 @@ static DBusMessage *hs_get_mic_gain(DBusConnection *conn,
dbus_uint16_t gain;

if (hs->state < HEADSET_STATE_CONNECTED || slc == NULL)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
- "Operation not Available");
+ return __btd_error_not_available(msg);

reply = dbus_message_new_method_return(msg);
if (!reply)
diff --git a/plugins/service.c b/plugins/service.c
index 6f6a275..f4b71ae 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -339,12 +339,6 @@ static void exit_callback(DBusConnection *conn, void *user_data)
g_free(user_record);
}

-static inline DBusMessage *not_available(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
- "Not Available");
-}
-
static inline DBusMessage *failed(DBusMessage *msg)
{
return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "Failed");
@@ -419,9 +413,7 @@ static DBusMessage *update_record(DBusConnection *conn, DBusMessage *msg,

if (remove_record_from_server(handle) < 0) {
sdp_record_free(sdp_record);
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".NotAvailable",
- "Not Available");
+ return __btd_error_not_available(msg);
}

if (serv_adapter->adapter)
@@ -465,9 +457,7 @@ static DBusMessage *update_xml_record(DBusConnection *conn,
user_record = find_record(serv_adapter, handle,
dbus_message_get_sender(msg));
if (!user_record)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".NotAvailable",
- "Not Available");
+ return __btd_error_not_available(msg);

sdp_record = sdp_xml_parse_record(record, len);
if (!sdp_record) {
@@ -552,7 +542,7 @@ static DBusMessage *remove_service_record(DBusConnection *conn,
sender = dbus_message_get_sender(msg);

if (remove_record(conn, sender, serv_adapter, handle) < 0)
- return not_available(msg);
+ return __btd_error_not_available(msg);

return dbus_message_new_method_return(msg);
}
diff --git a/src/error.c b/src/error.c
index 11bc520..8b91a03 100644
--- a/src/error.c
+++ b/src/error.c
@@ -83,3 +83,10 @@ DBusMessage *__btd_error_in_progress(DBusMessage *msg)
".InProgress",
"In Progress");
}
+
+DBusMessage *__btd_error_not_available(DBusMessage *msg)
+{
+ return g_dbus_create_error(msg, ERROR_INTERFACE
+ ".NotAvailable",
+ "Operation currently not available");
+}
diff --git a/src/error.h b/src/error.h
index e241b92..dfdc933 100644
--- a/src/error.h
+++ b/src/error.h
@@ -34,4 +34,5 @@ DBusMessage *__btd_error_invalid_args(DBusMessage *msg);
DBusMessage *__btd_error_already_exists(DBusMessage *msg);
DBusMessage *__btd_error_not_supported(DBusMessage *msg);
DBusMessage *__btd_error_not_connected(DBusMessage *msg);
+DBusMessage *__btd_error_not_available(DBusMessage *msg);
DBusMessage *__btd_error_in_progress(DBusMessage *msg);
--
1.7.3.1


2010-11-08 06:29:43

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 5/9] Add __btd_error_in_progress()

---
audio/device.c | 3 +--
audio/headset.c | 3 +--
input/device.c | 2 +-
src/device.c | 11 ++---------
src/error.c | 7 +++++++
src/error.h | 1 +
6 files changed, 13 insertions(+), 14 deletions(-)

diff --git a/audio/device.c b/audio/device.c
index 97badac..95fa51a 100644
--- a/audio/device.c
+++ b/audio/device.c
@@ -519,8 +519,7 @@ static DBusMessage *dev_connect(DBusConnection *conn, DBusMessage *msg,
struct dev_priv *priv = dev->priv;

if (priv->state == AUDIO_STATE_CONNECTING)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
- "Connect in Progress");
+ return __btd_error_in_progress(msg);
else if (priv->state == AUDIO_STATE_CONNECTED)
return g_dbus_create_error(msg, ERROR_INTERFACE
".AlreadyConnected",
diff --git a/audio/headset.c b/audio/headset.c
index 1ec48a4..fd358a0 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -1699,8 +1699,7 @@ static DBusMessage *hs_connect(DBusConnection *conn, DBusMessage *msg,
int err;

if (hs->state == HEADSET_STATE_CONNECTING)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
- "Connect in Progress");
+ return __btd_error_in_progress(msg);
else if (hs->state > HEADSET_STATE_CONNECTING)
return g_dbus_create_error(msg, ERROR_INTERFACE
".AlreadyConnected",
diff --git a/input/device.c b/input/device.c
index c85d89c..1ccb2b0 100644
--- a/input/device.c
+++ b/input/device.c
@@ -950,7 +950,7 @@ static DBusMessage *input_device_connect(DBusConnection *conn,
return not_supported(msg);

if (iconn->pending_connect)
- return in_progress(msg);
+ return __btd_error_in_progress(msg);

if (is_connected(iconn))
return already_connected(msg);
diff --git a/src/device.c b/src/device.c
index 3393634..f72a9b5 100644
--- a/src/device.c
+++ b/src/device.c
@@ -180,12 +180,6 @@ static inline DBusMessage *no_such_adapter(DBusMessage *msg)
"No such adapter");
}

-static inline DBusMessage *in_progress(DBusMessage *msg, const char *str)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
- "%s", str);
-}
-
static void browse_request_free(struct browse_req *req)
{
if (req->listener_id)
@@ -613,8 +607,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
int err;

if (device->browse)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InProgress",
- "Discover in progress");
+ return __btd_error_in_progress(msg);

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &pattern,
DBUS_TYPE_INVALID) == FALSE)
@@ -1933,7 +1926,7 @@ DBusMessage *device_create_bonding(struct btd_device *device,
ba2str(&device->bdaddr, dstaddr);

if (device->bonding)
- return in_progress(msg, "Bonding in progress");
+ return __btd_error_in_progress(msg);

/* check if a link key already exists */
create_name(filename, PATH_MAX, STORAGEDIR, srcaddr,
diff --git a/src/error.c b/src/error.c
index b2d0ced..11bc520 100644
--- a/src/error.c
+++ b/src/error.c
@@ -76,3 +76,10 @@ DBusMessage *__btd_error_not_connected(DBusMessage *msg)
".NotConnected",
"Not Connected");
}
+
+DBusMessage *__btd_error_in_progress(DBusMessage *msg)
+{
+ return g_dbus_create_error(msg, ERROR_INTERFACE
+ ".InProgress",
+ "In Progress");
+}
diff --git a/src/error.h b/src/error.h
index 06345da..e241b92 100644
--- a/src/error.h
+++ b/src/error.h
@@ -34,3 +34,4 @@ DBusMessage *__btd_error_invalid_args(DBusMessage *msg);
DBusMessage *__btd_error_already_exists(DBusMessage *msg);
DBusMessage *__btd_error_not_supported(DBusMessage *msg);
DBusMessage *__btd_error_not_connected(DBusMessage *msg);
+DBusMessage *__btd_error_in_progress(DBusMessage *msg);
--
1.7.3.1


2010-11-08 06:29:42

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 4/9] Add __btd_error_not_connected()

---
audio/control.c | 8 ++------
audio/device.c | 3 +--
audio/gateway.c | 4 +---
audio/headset.c | 24 ++++++------------------
audio/sink.c | 4 +---
audio/source.c | 4 +---
src/error.c | 7 +++++++
src/error.h | 1 +
8 files changed, 20 insertions(+), 35 deletions(-)

diff --git a/audio/control.c b/audio/control.c
index e9f59fc..043785e 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -1015,9 +1015,7 @@ static DBusMessage *volume_up(DBusConnection *conn, DBusMessage *msg,
return NULL;

if (control->state != AVCTP_STATE_CONNECTED)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);

if (!control->target)
return __btd_error_not_supported(msg);
@@ -1043,9 +1041,7 @@ static DBusMessage *volume_down(DBusConnection *conn, DBusMessage *msg,
return NULL;

if (control->state != AVCTP_STATE_CONNECTED)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);

if (!control->target)
return __btd_error_not_supported(msg);
diff --git a/audio/device.c b/audio/device.c
index eff8231..97badac 100644
--- a/audio/device.c
+++ b/audio/device.c
@@ -562,8 +562,7 @@ static DBusMessage *dev_disconnect(DBusConnection *conn, DBusMessage *msg,
struct dev_priv *priv = dev->priv;

if (priv->state == AUDIO_STATE_DISCONNECTED)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".NotConnected",
- "Not connected");
+ return __btd_error_not_connected(msg);

if (priv->dc_req)
return dbus_message_new_method_return(msg);
diff --git a/audio/gateway.c b/audio/gateway.c
index ae0ee75..ce1ec53 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -426,9 +426,7 @@ static DBusMessage *ag_disconnect(DBusConnection *conn, DBusMessage *msg,
return NULL;

if (!gw->rfcomm)
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);

gateway_close(device);
ba2str(&device->dst, gw_addr);
diff --git a/audio/headset.c b/audio/headset.c
index 2cca5ca..1ec48a4 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -1622,9 +1622,7 @@ static DBusMessage *hs_stop(DBusConnection *conn, DBusMessage *msg,
DBusMessage *reply = NULL;

if (hs->state < HEADSET_STATE_PLAY_IN_PROGRESS)
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);

reply = dbus_message_new_method_return(msg);
if (!reply)
@@ -1663,9 +1661,7 @@ static DBusMessage *hs_disconnect(DBusConnection *conn, DBusMessage *msg,
char hs_address[18];

if (hs->state == HEADSET_STATE_DISCONNECTED)
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);

headset_shutdown(device);
ba2str(&device->dst, hs_address);
@@ -1741,9 +1737,7 @@ static DBusMessage *hs_ring(DBusConnection *conn, DBusMessage *msg,
int err;

if (hs->state < HEADSET_STATE_CONNECTED)
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);

reply = dbus_message_new_method_return(msg);
if (!reply)
@@ -1778,9 +1772,7 @@ static DBusMessage *hs_cancel_call(DBusConnection *conn,
DBusMessage *reply = NULL;

if (hs->state < HEADSET_STATE_CONNECTED)
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);

reply = dbus_message_new_method_return(msg);
if (!reply)
@@ -1812,9 +1804,7 @@ static DBusMessage *hs_play(DBusConnection *conn, DBusMessage *msg,
switch (hs->state) {
case HEADSET_STATE_DISCONNECTED:
case HEADSET_STATE_CONNECTING:
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);
case HEADSET_STATE_PLAY_IN_PROGRESS:
if (hs->pending && hs->pending->msg == NULL) {
hs->pending->msg = dbus_message_ref(msg);
@@ -1905,9 +1895,7 @@ static DBusMessage *hs_set_gain(DBusConnection *conn,
int err;

if (hs->state < HEADSET_STATE_CONNECTED)
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);

err = headset_set_gain(device, gain, type);
if (err < 0)
diff --git a/audio/sink.c b/audio/sink.c
index a9f6307..3c55c74 100644
--- a/audio/sink.c
+++ b/audio/sink.c
@@ -456,9 +456,7 @@ static DBusMessage *sink_disconnect(DBusConnection *conn,
int err;

if (!sink->session)
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);

if (sink->connect || sink->disconnect)
return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
diff --git a/audio/source.c b/audio/source.c
index 8258fcd..4d47853 100644
--- a/audio/source.c
+++ b/audio/source.c
@@ -423,9 +423,7 @@ static DBusMessage *source_disconnect(DBusConnection *conn,
int err;

if (!source->session)
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".NotConnected",
- "Device not Connected");
+ return __btd_error_not_connected(msg);

if (source->connect || source->disconnect)
return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
diff --git a/src/error.c b/src/error.c
index 4429903..b2d0ced 100644
--- a/src/error.c
+++ b/src/error.c
@@ -69,3 +69,10 @@ DBusMessage *__btd_error_not_supported(DBusMessage *msg)
".NotSupported",
"Operation is not supported");
}
+
+DBusMessage *__btd_error_not_connected(DBusMessage *msg)
+{
+ return g_dbus_create_error(msg, ERROR_INTERFACE
+ ".NotConnected",
+ "Not Connected");
+}
diff --git a/src/error.h b/src/error.h
index 7dbb8b3..06345da 100644
--- a/src/error.h
+++ b/src/error.h
@@ -33,3 +33,4 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
DBusMessage *__btd_error_invalid_args(DBusMessage *msg);
DBusMessage *__btd_error_already_exists(DBusMessage *msg);
DBusMessage *__btd_error_not_supported(DBusMessage *msg);
+DBusMessage *__btd_error_not_connected(DBusMessage *msg);
--
1.7.3.1


2010-11-08 06:29:41

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 3/9] Add __btd_error_not_supported()

---
audio/control.c | 8 ++------
src/device.c | 4 +---
src/error.c | 7 +++++++
src/error.h | 1 +
4 files changed, 11 insertions(+), 9 deletions(-)

diff --git a/audio/control.c b/audio/control.c
index 76f46e9..e9f59fc 100644
--- a/audio/control.c
+++ b/audio/control.c
@@ -1020,9 +1020,7 @@ static DBusMessage *volume_up(DBusConnection *conn, DBusMessage *msg,
"Device not Connected");

if (!control->target)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".NotSupported",
- "AVRCP Target role not supported");
+ return __btd_error_not_supported(msg);

err = avctp_send_passthrough(control, VOL_UP_OP);
if (err < 0)
@@ -1050,9 +1048,7 @@ static DBusMessage *volume_down(DBusConnection *conn, DBusMessage *msg,
"Device not Connected");

if (!control->target)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".NotSupported",
- "AVRCP Target role not supported");
+ return __btd_error_not_supported(msg);

err = avctp_send_passthrough(control, VOL_DOWN_OP);
if (err < 0)
diff --git a/src/device.c b/src/device.c
index 6d110dc..3393634 100644
--- a/src/device.c
+++ b/src/device.c
@@ -540,9 +540,7 @@ static DBusMessage *set_blocked(DBusConnection *conn, DBusMessage *msg,
case 0:
return dbus_message_new_method_return(msg);
case EINVAL:
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".NotSupported",
- "Kernel lacks blacklist support");
+ return __btd_error_not_supported(msg);
default:
return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
"%s", strerror(-err));
diff --git a/src/error.c b/src/error.c
index e268163..4429903 100644
--- a/src/error.c
+++ b/src/error.c
@@ -62,3 +62,10 @@ DBusMessage *__btd_error_already_exists(DBusMessage *msg)
ERROR_INTERFACE ".AlreadyExists",
"Already Exists");
}
+
+DBusMessage *__btd_error_not_supported(DBusMessage *msg)
+{
+ return g_dbus_create_error(msg, ERROR_INTERFACE
+ ".NotSupported",
+ "Operation is not supported");
+}
diff --git a/src/error.h b/src/error.h
index 284889f..7dbb8b3 100644
--- a/src/error.h
+++ b/src/error.h
@@ -32,3 +32,4 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,

DBusMessage *__btd_error_invalid_args(DBusMessage *msg);
DBusMessage *__btd_error_already_exists(DBusMessage *msg);
+DBusMessage *__btd_error_not_supported(DBusMessage *msg);
--
1.7.3.1


2010-11-08 06:29:39

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 1/9] Create __btd_error_invalid_args()

DBus error handling in BlueZ is a mess. This is the first patch to unify
all DBus error handling like in ConnMan and oFono. This unifies all
.InvalidArguments errors.
---
attrib/client.c | 20 +++++----------
audio/gateway.c | 8 +----
audio/headset.c | 18 ++++---------
audio/media.c | 9 ++----
audio/telephony-dummy.c | 25 ++++++++------------
audio/telephony-maemo5.c | 11 ++------
audio/telephony-maemo6.c | 11 ++------
audio/transport.c | 14 +++--------
health/hdp.c | 58 ++++++++++++----------------------------------
network/server.c | 7 -----
plugins/service.c | 8 +-----
serial/port.c | 8 ------
serial/proxy.c | 19 ++++----------
src/adapter.c | 52 ++++++++++++++++++-----------------------
src/device.c | 22 ++++++-----------
src/error.c | 7 +++++
src/error.h | 2 +
src/manager.c | 7 -----
18 files changed, 100 insertions(+), 206 deletions(-)

diff --git a/attrib/client.c b/attrib/client.c
index 1f2c217..cd6e401 100644
--- a/attrib/client.c
+++ b/attrib/client.c
@@ -190,12 +190,6 @@ static int watcher_cmp(gconstpointer a, gconstpointer b)
return g_strcmp0(watcher->path, match->path);
}

-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
-}
-
static inline DBusMessage *not_authorized(DBusMessage *msg)
{
return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAuthorized",
@@ -465,7 +459,7 @@ static DBusMessage *register_watcher(DBusConnection *conn,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (l2cap_connect(prim->gatt, &gerr, TRUE) < 0) {
DBusMessage *reply;
@@ -499,7 +493,7 @@ static DBusMessage *unregister_watcher(DBusConnection *conn,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

match = g_new0(struct watcher, 1);
match->name = g_strdup(sender);
@@ -537,7 +531,7 @@ static DBusMessage *set_value(DBusConnection *conn, DBusMessage *msg,

if (dbus_message_iter_get_arg_type(iter) != DBUS_TYPE_ARRAY ||
dbus_message_iter_get_element_type(iter) != DBUS_TYPE_BYTE)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_recurse(iter, &sub);

@@ -586,23 +580,23 @@ static DBusMessage *set_property(DBusConnection *conn,
const char *property;

if (!dbus_message_iter_init(msg, &iter))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&iter, &property);
dbus_message_iter_next(&iter);

if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_recurse(&iter, &sub);

if (g_str_equal("Value", property))
return set_value(conn, msg, &sub, chr);

- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
}

static GDBusMethodTable char_methods[] = {
diff --git a/audio/gateway.c b/audio/gateway.c
index 07ebdd4..ab7d310 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -494,9 +494,7 @@ static DBusMessage *register_agent(DBusConnection *conn,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid argument");
+ return __btd_error_invalid_args(msg);

name = dbus_message_get_sender(msg);
agent = g_new0(struct hf_agent, 1);
@@ -529,9 +527,7 @@ static DBusMessage *unregister_agent(DBusConnection *conn,
if (!dbus_message_get_args(msg, NULL,
DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid argument");
+ return __btd_error_invalid_args(msg);

if (strcmp(gw->agent->path, path) != 0)
return g_dbus_create_error(msg,
diff --git a/audio/headset.c b/audio/headset.c
index 0763585..2cca5ca 100644
--- a/audio/headset.c
+++ b/audio/headset.c
@@ -180,12 +180,6 @@ struct event {

static GSList *headset_callbacks = NULL;

-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
-}
-
static DBusHandlerResult error_not_supported(DBusConnection *conn,
DBusMessage *msg)
{
@@ -2026,35 +2020,35 @@ static DBusMessage *hs_set_property(DBusConnection *conn,
uint16_t gain;

if (!dbus_message_iter_init(msg, &iter))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&iter, &property);
dbus_message_iter_next(&iter);

if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
dbus_message_iter_recurse(&iter, &sub);

if (g_str_equal("SpeakerGain", property)) {
if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT16)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&sub, &gain);
return hs_set_gain(conn, msg, data, gain,
HEADSET_GAIN_SPEAKER);
} else if (g_str_equal("MicrophoneGain", property)) {
if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT16)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&sub, &gain);
return hs_set_gain(conn, msg, data, gain,
HEADSET_GAIN_MICROPHONE);
}

- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
}
static GDBusMethodTable headset_methods[] = {
{ "Connect", "", "", hs_connect,
diff --git a/audio/media.c b/audio/media.c
index b6c90f9..862cee6 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -323,18 +323,15 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,

dbus_message_iter_recurse(&args, &props);
if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
- return g_dbus_create_error(msg, ERROR_INTERFACE
- ".Failed", "Invalid argument");
+ return __btd_error_invalid_args(msg);

if (parse_properties(&props, &uuid, &delay_reporting, &codec,
&capabilities, &size) || uuid == NULL)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
- "Invalid argument");
+ return __btd_error_invalid_args(msg);

if (media_endpoint_create(adapter, sender, path, uuid, delay_reporting,
codec, capabilities, size) == FALSE)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
- "Invalid argument");
+ return __btd_error_invalid_args(msg);

return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
}
diff --git a/audio/telephony-dummy.c b/audio/telephony-dummy.c
index 06cb798..b56b6e7 100644
--- a/audio/telephony-dummy.c
+++ b/audio/telephony-dummy.c
@@ -35,6 +35,7 @@

#include "log.h"
#include "telephony.h"
+#include "error.h"

#define TELEPHONY_DUMMY_IFACE "org.bluez.TelephonyTest"
#define TELEPHONY_DUMMY_PATH "/org/bluez/test"
@@ -69,12 +70,6 @@ static struct indicator dummy_indicators[] =
{ NULL }
};

-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, "org.bluez.Error.InvalidArguments",
- "Invalid arguments in method call");
-}
-
void telephony_device_connected(void *telephony_device)
{
DBG("telephony-dummy: device %p connected", telephony_device);
@@ -236,7 +231,7 @@ static DBusMessage *outgoing_call(DBusConnection *conn, DBusMessage *msg,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
DBUS_TYPE_INVALID))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

DBG("telephony-dummy: outgoing call to %s", number);

@@ -261,7 +256,7 @@ static DBusMessage *incoming_call(DBusConnection *conn, DBusMessage *msg,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
DBUS_TYPE_INVALID))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

DBG("telephony-dummy: incoming call to %s", number);

@@ -307,10 +302,10 @@ static DBusMessage *signal_strength(DBusConnection *conn, DBusMessage *msg,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UINT32, &strength,
DBUS_TYPE_INVALID))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (strength > 5)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

telephony_update_indicator(dummy_indicators, "signal", strength);

@@ -326,10 +321,10 @@ static DBusMessage *battery_level(DBusConnection *conn, DBusMessage *msg,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_UINT32, &level,
DBUS_TYPE_INVALID))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (level > 5)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

telephony_update_indicator(dummy_indicators, "battchg", level);

@@ -346,7 +341,7 @@ static DBusMessage *roaming_status(DBusConnection *conn, DBusMessage *msg,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_BOOLEAN, &roaming,
DBUS_TYPE_INVALID))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

val = roaming ? EV_ROAM_ACTIVE : EV_ROAM_INACTIVE;

@@ -365,7 +360,7 @@ static DBusMessage *registration_status(DBusConnection *conn, DBusMessage *msg,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_BOOLEAN, &registration,
DBUS_TYPE_INVALID))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

val = registration ? EV_SERVICE_PRESENT : EV_SERVICE_NONE;

@@ -384,7 +379,7 @@ static DBusMessage *set_subscriber_number(DBusConnection *conn,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &number,
DBUS_TYPE_INVALID))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

g_free(subscriber_number);
subscriber_number = g_strdup(number);
diff --git a/audio/telephony-maemo5.c b/audio/telephony-maemo5.c
index 4d0134c..0f9819c 100644
--- a/audio/telephony-maemo5.c
+++ b/audio/telephony-maemo5.c
@@ -38,6 +38,7 @@

#include "log.h"
#include "telephony.h"
+#include "error.h"

/* SSC D-Bus definitions */
#define SSC_DBUS_NAME "com.nokia.phone.SSC"
@@ -1880,12 +1881,6 @@ static void csd_init(void)
}
}

-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
- return g_dbus_create_error(msg,"org.bluez.Error.InvalidArguments",
- "Invalid arguments in method call");
-}
-
static uint32_t get_callflag(const char *callerid_setting)
{
if (callerid_setting != NULL) {
@@ -1950,7 +1945,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING,
&callerid_setting,
DBUS_TYPE_INVALID) == FALSE)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (g_str_equal(callerid_setting, "allowed") ||
g_str_equal(callerid_setting, "restricted") ||
@@ -1964,7 +1959,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,

error("telephony-maemo: invalid argument %s for method call"
" SetCallerId", callerid_setting);
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
}

static GDBusMethodTable telephony_maemo_methods[] = {
diff --git a/audio/telephony-maemo6.c b/audio/telephony-maemo6.c
index 72c8e36..4663b4d 100644
--- a/audio/telephony-maemo6.c
+++ b/audio/telephony-maemo6.c
@@ -38,6 +38,7 @@

#include "log.h"
#include "telephony.h"
+#include "error.h"

/* SSC D-Bus definitions */
#define SSC_DBUS_NAME "com.nokia.phone.SSC"
@@ -1760,12 +1761,6 @@ static void csd_init(void)
}
}

-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
- return g_dbus_create_error(msg,"org.bluez.Error.InvalidArguments",
- "Invalid arguments in method call");
-}
-
static uint32_t get_callflag(const char *callerid_setting)
{
if (callerid_setting != NULL) {
@@ -1830,7 +1825,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,
if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING,
&callerid_setting,
DBUS_TYPE_INVALID) == FALSE)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (g_str_equal(callerid_setting, "allowed") ||
g_str_equal(callerid_setting, "restricted") ||
@@ -1844,7 +1839,7 @@ static DBusMessage *set_callerid(DBusConnection *conn, DBusMessage *msg,

error("telephony-maemo6: invalid argument %s for method call"
" SetCallerId", callerid_setting);
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
}

static DBusMessage *clear_lastnumber(DBusConnection *conn, DBusMessage *msg,
diff --git a/audio/transport.c b/audio/transport.c
index eda46e1..0c865f7 100644
--- a/audio/transport.c
+++ b/audio/transport.c
@@ -93,12 +93,6 @@ struct media_transport {
DBusMessageIter *value);
};

-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
-}
-
static inline DBusMessage *error_failed(DBusMessage *msg, const char *desc)
{
return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed", "%s", desc);
@@ -549,16 +543,16 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,
int err;

if (!dbus_message_iter_init(msg, &iter))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&iter, &property);
dbus_message_iter_next(&iter);

if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
dbus_message_iter_recurse(&iter, &value);

sender = dbus_message_get_sender(msg);
@@ -577,7 +571,7 @@ static DBusMessage *set_property(DBusConnection *conn, DBusMessage *msg,

if (err < 0) {
if (err == -EINVAL)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
return error_failed(msg, strerror(-err));
}

diff --git a/health/hdp.c b/health/hdp.c
index 1eba8e1..6c1fd98 100644
--- a/health/hdp.c
+++ b/health/hdp.c
@@ -321,15 +321,8 @@ static DBusMessage *manager_create_application(DBusConnection *conn,

dbus_message_iter_init(msg, &iter);
app = hdp_get_app_config(&iter, &err);
- if (err) {
- DBusMessage *reply;
-
- reply = g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments: %s", err->message);
- g_error_free(err);
- return reply;
- }
+ if (err)
+ return __btd_error_invalid_args(msg);

name = dbus_message_get_sender(msg);
if (!name) {
@@ -368,11 +361,8 @@ static DBusMessage *manager_destroy_application(DBusConnection *conn,
GSList *l;

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
- DBUS_TYPE_INVALID)){
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
- }
+ DBUS_TYPE_INVALID))
+ return __btd_error_invalid_args(msg);

l = g_slist_find_custom(applications, path, cmp_app);

@@ -1801,18 +1791,13 @@ static DBusMessage *device_create_channel(DBusConnection *conn,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &app_path,
DBUS_TYPE_STRING, &conf,
- DBUS_TYPE_INVALID)) {
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
- }
+ DBUS_TYPE_INVALID))
+ return __btd_error_invalid_args(msg);

l = g_slist_find_custom(applications, app_path, cmp_app);
if (!l)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call, "
- "no such application");
+ return __btd_error_invalid_args(msg);
+
app = l->data;

if (g_ascii_strcasecmp("Reliable", conf) == 0)
@@ -1822,25 +1807,16 @@ static DBusMessage *device_create_channel(DBusConnection *conn,
else if (g_ascii_strcasecmp("Any", conf) == 0)
config = HDP_NO_PREFERENCE_DC;
else
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
+ return __btd_error_invalid_args(msg);

if (app->role == HDP_SINK && config != HDP_NO_PREFERENCE_DC)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Configuration not valid for sinks");
+ return __btd_error_invalid_args(msg);

if (app->role == HDP_SOURCE && config == HDP_NO_PREFERENCE_DC)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Configuration not valid for sources");
+ return __btd_error_invalid_args(msg);

if (!device->fr && config == HDP_STREAMING_DC)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Configuration not valid, first "
- "channel should be reliable");
+ return __btd_error_invalid_args(msg);

data = g_new0(struct hdp_create_dc, 1);
data->dev = device;
@@ -1924,17 +1900,13 @@ static DBusMessage *device_destroy_channel(DBusConnection *conn,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID)){
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
+ return __btd_error_invalid_args(msg);
}

l = g_slist_find_custom(device->channels, path, cmp_chan_path);
if (!l)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call, "
- "no such channel");
+ return __btd_error_invalid_args(msg);
+
hdp_chan = l->data;
del_data = g_new0(struct hdp_tmp_dc_data, 1);
del_data->msg = dbus_message_ref(msg);
diff --git a/network/server.c b/network/server.c
index e6706ec..ce1fe5e 100644
--- a/network/server.c
+++ b/network/server.c
@@ -575,13 +575,6 @@ static inline DBusMessage *failed(DBusMessage *msg, const char *description)
"%s", description);
}

-static inline DBusMessage *invalid_arguments(DBusMessage *msg,
- const char *description)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
- "%s", description);
-}
-
static void server_disconnect(DBusConnection *conn, void *user_data)
{
struct network_server *ns = user_data;
diff --git a/plugins/service.c b/plugins/service.c
index f6b9da7..6f6a275 100644
--- a/plugins/service.c
+++ b/plugins/service.c
@@ -339,12 +339,6 @@ static void exit_callback(DBusConnection *conn, void *user_data)
g_free(user_record);
}

-static inline DBusMessage *invalid_arguments(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
-}
-
static inline DBusMessage *not_available(DBusMessage *msg)
{
return g_dbus_create_error(msg, ERROR_INTERFACE ".NotAvailable",
@@ -466,7 +460,7 @@ static DBusMessage *update_xml_record(DBusConnection *conn,

len = (record ? strlen(record) : 0);
if (len == 0)
- return invalid_arguments(msg);
+ return __btd_error_invalid_args(msg);

user_record = find_record(serv_adapter, handle,
dbus_message_get_sender(msg));
diff --git a/serial/port.c b/serial/port.c
index 0398f2e..b593311 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -57,7 +57,6 @@
#include "port.h"

#define SERIAL_PORT_INTERFACE "org.bluez.Serial"
-#define ERROR_INVALID_ARGS "org.bluez.Error.InvalidArguments"
#define ERROR_DOES_NOT_EXIST "org.bluez.Error.DoesNotExist"

#define MAX_OPEN_TRIES 5
@@ -243,13 +242,6 @@ static inline DBusMessage *does_not_exist(DBusMessage *msg,
"%s", description);
}

-static inline DBusMessage *invalid_arguments(DBusMessage *msg,
- const char *description)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
- "%s", description);
-}
-
static inline DBusMessage *failed(DBusMessage *msg, const char *description)
{
return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
diff --git a/serial/proxy.c b/serial/proxy.c
index 3aaabf3..8e182b6 100644
--- a/serial/proxy.c
+++ b/serial/proxy.c
@@ -140,13 +140,6 @@ static inline DBusMessage *does_not_exist(DBusMessage *msg,
"%s", description);
}

-static inline DBusMessage *invalid_arguments(DBusMessage *msg,
- const char *description)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
- "%s", description);
-}
-
static inline DBusMessage *failed(DBusMessage *msg, const char *description)
{
return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
@@ -755,17 +748,17 @@ static DBusMessage *proxy_set_serial_params(DBusConnection *conn,
return NULL;

if (str2speed(ratestr, &speed) == B0)
- return invalid_arguments(msg, "Invalid baud rate");
+ return __btd_error_invalid_args(msg);

ctrl = prx->proxy_ti.c_cflag;
if (set_databits(databits, &ctrl) < 0)
- return invalid_arguments(msg, "Invalid data bits");
+ return __btd_error_invalid_args(msg);

if (set_stopbits(stopbits, &ctrl) < 0)
- return invalid_arguments(msg, "Invalid stop bits");
+ return __btd_error_invalid_args(msg);

if (set_parity(paritystr, &ctrl) < 0)
- return invalid_arguments(msg, "Invalid parity");
+ return __btd_error_invalid_args(msg);

prx->proxy_ti.c_cflag = ctrl;
prx->proxy_ti.c_cflag |= (CLOCAL | CREAD);
@@ -1057,13 +1050,13 @@ static DBusMessage *create_proxy(DBusConnection *conn,

uuid_str = bt_name2string(pattern);
if (!uuid_str)
- return invalid_arguments(msg, "Invalid UUID");
+ return __btd_error_invalid_args(msg);

err = register_proxy(adapter, uuid_str, address, &proxy);
g_free(uuid_str);

if (err == -EINVAL)
- return invalid_arguments(msg, "Invalid address");
+ return __btd_error_invalid_args(msg);
else if (err == -EALREADY)
return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExist",
"Proxy already exists");
diff --git a/src/adapter.c b/src/adapter.c
index 31014e5..cc51816 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -148,12 +148,6 @@ struct btd_adapter {
static void adapter_set_pairable_timeout(struct btd_adapter *adapter,
guint interval);

-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
-}
-
static inline DBusMessage *adapter_not_ready(DBusMessage *msg)
{
return g_dbus_create_error(msg, ERROR_INTERFACE ".NotReady",
@@ -1088,7 +1082,7 @@ static DBusMessage *set_name(DBusConnection *conn, DBusMessage *msg,

if (!g_utf8_validate(name, -1, NULL)) {
error("Name change failed: supplied name isn't valid UTF-8");
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
}

if (strncmp(name, (char *) dev->name, MAX_NAME_LENGTH) == 0)
@@ -1535,23 +1529,23 @@ static DBusMessage *set_property(DBusConnection *conn,
ba2str(&adapter->bdaddr, srcaddr);

if (!dbus_message_iter_init(msg, &iter))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&iter, &property);
dbus_message_iter_next(&iter);

if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
dbus_message_iter_recurse(&iter, &sub);

if (g_str_equal("Name", property)) {
const char *name;

if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_STRING)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
dbus_message_iter_get_basic(&sub, &name);

return set_name(conn, msg, name, data);
@@ -1559,7 +1553,7 @@ static DBusMessage *set_property(DBusConnection *conn,
gboolean powered;

if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&sub, &powered);

@@ -1568,7 +1562,7 @@ static DBusMessage *set_property(DBusConnection *conn,
gboolean discoverable;

if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&sub, &discoverable);

@@ -1577,7 +1571,7 @@ static DBusMessage *set_property(DBusConnection *conn,
uint32_t timeout;

if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT32)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&sub, &timeout);

@@ -1586,7 +1580,7 @@ static DBusMessage *set_property(DBusConnection *conn,
gboolean pairable;

if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&sub, &pairable);

@@ -1595,14 +1589,14 @@ static DBusMessage *set_property(DBusConnection *conn,
uint32_t timeout;

if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_UINT32)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&sub, &timeout);

return set_pairable_timeout(conn, msg, timeout, data);
}

- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
}

static DBusMessage *request_session(DBusConnection *conn,
@@ -1676,7 +1670,7 @@ static DBusMessage *list_devices(DBusConnection *conn,
const gchar *dev_path;

if (!dbus_message_has_signature(msg, DBUS_TYPE_INVALID_AS_STRING))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

reply = dbus_message_new_method_return(msg);
if (!reply)
@@ -1709,10 +1703,10 @@ static DBusMessage *cancel_device_creation(DBusConnection *conn,

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_INVALID) == FALSE)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (check_address(address) < 0)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

device = adapter_find_device(adapter, address);
if (!device || !device_is_creating(device, NULL))
@@ -1742,10 +1736,10 @@ static DBusMessage *create_device(DBusConnection *conn,

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_INVALID) == FALSE)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (check_address(address) < 0)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (adapter_find_device(adapter, address))
return g_dbus_create_error(msg,
@@ -1790,21 +1784,21 @@ static DBusMessage *create_paired_device(DBusConnection *conn,
DBUS_TYPE_OBJECT_PATH, &agent_path,
DBUS_TYPE_STRING, &capability,
DBUS_TYPE_INVALID) == FALSE)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (check_address(address) < 0)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

sender = dbus_message_get_sender(msg);
if (adapter->agent &&
agent_matches(adapter->agent, sender, agent_path)) {
error("Refusing adapter agent usage as device specific one");
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
}

cap = parse_io_capability(capability);
if (cap == IO_CAPABILITY_INVALID)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

device = adapter_get_device(conn, adapter, address);
if (!device)
@@ -1832,7 +1826,7 @@ static DBusMessage *remove_device(DBusConnection *conn, DBusMessage *msg,

if (dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID) == FALSE)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

l = g_slist_find_custom(adapter->devices,
path, (GCompareFunc) device_path_cmp);
@@ -1870,7 +1864,7 @@ static DBusMessage *find_device(DBusConnection *conn, DBusMessage *msg,

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &address,
DBUS_TYPE_INVALID))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

l = g_slist_find_custom(adapter->devices,
address, (GCompareFunc) device_address_cmp);
@@ -1918,7 +1912,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,

cap = parse_io_capability(capability);
if (cap == IO_CAPABILITY_INVALID)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

name = dbus_message_get_sender(msg);

diff --git a/src/device.c b/src/device.c
index c2b6682..ef1a910 100644
--- a/src/device.c
+++ b/src/device.c
@@ -549,12 +549,6 @@ static DBusMessage *set_blocked(DBusConnection *conn, DBusMessage *msg,
}
}

-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
- return g_dbus_create_error(msg, ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
-}
-
static DBusMessage *set_property(DBusConnection *conn,
DBusMessage *msg, void *data)
{
@@ -563,22 +557,22 @@ static DBusMessage *set_property(DBusConnection *conn,
const char *property;

if (!dbus_message_iter_init(msg, &iter))
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_STRING)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&iter, &property);
dbus_message_iter_next(&iter);

if (dbus_message_iter_get_arg_type(&iter) != DBUS_TYPE_VARIANT)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
dbus_message_iter_recurse(&iter, &sub);

if (g_str_equal("Trusted", property)) {
dbus_bool_t value;
if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
dbus_message_iter_get_basic(&sub, &value);

return set_trust(conn, msg, value, data);
@@ -586,7 +580,7 @@ static DBusMessage *set_property(DBusConnection *conn,
const char *alias;

if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_STRING)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
dbus_message_iter_get_basic(&sub, &alias);

return set_alias(conn, msg, alias, data);
@@ -594,14 +588,14 @@ static DBusMessage *set_property(DBusConnection *conn,
dbus_bool_t value;

if (dbus_message_iter_get_arg_type(&sub) != DBUS_TYPE_BOOLEAN)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

dbus_message_iter_get_basic(&sub, &value);

return set_blocked(conn, msg, value, data);
}

- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);
}

static void discover_services_req_exit(DBusConnection *conn, void *user_data)
@@ -636,7 +630,7 @@ static DBusMessage *discover_services(DBusConnection *conn,
uuid_t uuid;

if (bt_string2uuid(&uuid, pattern) < 0)
- return invalid_args(msg);
+ return __btd_error_invalid_args(msg);

sdp_uuid128_to_uuid(&uuid);

diff --git a/src/error.c b/src/error.c
index c1a2fbf..a30c050 100644
--- a/src/error.c
+++ b/src/error.c
@@ -48,3 +48,10 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,

return DBUS_HANDLER_RESULT_HANDLED;
}
+
+DBusMessage *__btd_error_invalid_args(DBusMessage *msg)
+{
+ return g_dbus_create_error(msg, ERROR_INTERFACE
+ ".InvalidArguments",
+ "Invalid arguments in method call");
+}
diff --git a/src/error.h b/src/error.h
index 49ec05e..51c094c 100644
--- a/src/error.h
+++ b/src/error.h
@@ -29,3 +29,5 @@

DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
const char *name, const char *descr);
+
+DBusMessage *__btd_error_invalid_args(DBusMessage *msg);
diff --git a/src/manager.c b/src/manager.c
index 3fad9e6..315f5c3 100644
--- a/src/manager.c
+++ b/src/manager.c
@@ -66,13 +66,6 @@ void manager_update_svc(struct btd_adapter* adapter, uint8_t svc)
adapter_set_service_classes(adapter, svc);
}

-static inline DBusMessage *invalid_args(DBusMessage *msg)
-{
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".InvalidArguments",
- "Invalid arguments in method call");
-}
-
static inline DBusMessage *no_such_adapter(DBusMessage *msg)
{
return g_dbus_create_error(msg,
--
1.7.3.1


2010-11-08 06:29:40

by Gustavo Padovan

[permalink] [raw]
Subject: [PATCH 2/9] Add __btd_error_already_exists()

---
audio/gateway.c | 4 +---
audio/media.c | 3 +--
network/server.c | 2 +-
serial/proxy.c | 3 +--
src/adapter.c | 8 ++------
src/device.c | 4 +---
src/error.c | 7 +++++++
src/error.h | 1 +
8 files changed, 15 insertions(+), 17 deletions(-)

diff --git a/audio/gateway.c b/audio/gateway.c
index ab7d310..ae0ee75 100644
--- a/audio/gateway.c
+++ b/audio/gateway.c
@@ -488,9 +488,7 @@ static DBusMessage *register_agent(DBusConnection *conn,
const char *path, *name;

if (gw->agent)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".AlreadyExists",
- "Agent already exists");
+ return __btd_error_already_exists(msg);

if (!dbus_message_get_args(msg, NULL, DBUS_TYPE_OBJECT_PATH, &path,
DBUS_TYPE_INVALID))
diff --git a/audio/media.c b/audio/media.c
index 862cee6..8821ee1 100644
--- a/audio/media.c
+++ b/audio/media.c
@@ -318,8 +318,7 @@ static DBusMessage *register_endpoint(DBusConnection *conn, DBusMessage *msg,
dbus_message_iter_next(&args);

if (media_adapter_find_endpoint(adapter, sender, path, NULL) != NULL)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".Failed",
- "Endpoint already registered");
+ return __btd_error_already_exists(msg);

dbus_message_iter_recurse(&args, &props);
if (dbus_message_iter_get_arg_type(&props) != DBUS_TYPE_DICT_ENTRY)
diff --git a/network/server.c b/network/server.c
index ce1fe5e..41c9ec3 100644
--- a/network/server.c
+++ b/network/server.c
@@ -605,7 +605,7 @@ static DBusMessage *register_server(DBusConnection *conn,
return failed(msg, "Invalid UUID");

if (ns->record_id)
- return failed(msg, "Already registered");
+ return __btd_error_already_exists(msg);

reply = dbus_message_new_method_return(msg);
if (!reply)
diff --git a/serial/proxy.c b/serial/proxy.c
index 8e182b6..de82f9a 100644
--- a/serial/proxy.c
+++ b/serial/proxy.c
@@ -1058,8 +1058,7 @@ static DBusMessage *create_proxy(DBusConnection *conn,
if (err == -EINVAL)
return __btd_error_invalid_args(msg);
else if (err == -EALREADY)
- return g_dbus_create_error(msg, ERROR_INTERFACE ".AlreadyExist",
- "Proxy already exists");
+ return __btd_error_already_exists(msg);
else if (err < 0)
return g_dbus_create_error(msg, ERROR_INTERFACE "Failed",
"Proxy creation failed (%s)", strerror(-err));
diff --git a/src/adapter.c b/src/adapter.c
index cc51816..ffbc943 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -1742,9 +1742,7 @@ static DBusMessage *create_device(DBusConnection *conn,
return __btd_error_invalid_args(msg);

if (adapter_find_device(adapter, address))
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".AlreadyExists",
- "Device already exists");
+ return __btd_error_already_exists(msg);

DBG("%s", address);

@@ -1906,9 +1904,7 @@ static DBusMessage *register_agent(DBusConnection *conn, DBusMessage *msg,
return NULL;

if (adapter->agent)
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".AlreadyExists",
- "Agent already exists");
+ return __btd_error_already_exists(msg);

cap = parse_io_capability(capability);
if (cap == IO_CAPABILITY_INVALID)
diff --git a/src/device.c b/src/device.c
index ef1a910..6d110dc 100644
--- a/src/device.c
+++ b/src/device.c
@@ -1944,9 +1944,7 @@ DBusMessage *device_create_bonding(struct btd_device *device,
str = textfile_caseget(filename, dstaddr);
if (str) {
free(str);
- return g_dbus_create_error(msg,
- ERROR_INTERFACE ".AlreadyExists",
- "Bonding already exists");
+ return __btd_error_already_exists(msg);
}

/* If our IO capability is NoInputNoOutput use medium security
diff --git a/src/error.c b/src/error.c
index a30c050..e268163 100644
--- a/src/error.c
+++ b/src/error.c
@@ -55,3 +55,10 @@ DBusMessage *__btd_error_invalid_args(DBusMessage *msg)
".InvalidArguments",
"Invalid arguments in method call");
}
+
+DBusMessage *__btd_error_already_exists(DBusMessage *msg)
+{
+ return g_dbus_create_error(msg,
+ ERROR_INTERFACE ".AlreadyExists",
+ "Already Exists");
+}
diff --git a/src/error.h b/src/error.h
index 51c094c..284889f 100644
--- a/src/error.h
+++ b/src/error.h
@@ -31,3 +31,4 @@ DBusHandlerResult error_common_reply(DBusConnection *conn, DBusMessage *msg,
const char *name, const char *descr);

DBusMessage *__btd_error_invalid_args(DBusMessage *msg);
+DBusMessage *__btd_error_already_exists(DBusMessage *msg);
--
1.7.3.1