* [PATCH 1/2] Avoid possible memory leak in mcap_connect_mdl function
From: Jose Antonio Santos Cadenas @ 2010-09-06 8:11 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Jose Antonio Santos Cadenas
---
health/mcap.c | 25 +++++++++++++++++++++++--
1 files changed, 23 insertions(+), 2 deletions(-)
diff --git a/health/mcap.c b/health/mcap.c
index 6f1e565..30a1e6e 100644
--- a/health/mcap.c
+++ b/health/mcap.c
@@ -65,6 +65,7 @@ struct mcap_mdl_op_cb {
struct mcap_mdl *mdl; /* MDL for this operation */
mcap_cb_type cb; /* Operation callback */
gpointer user_data; /* Callback user data */
+ gboolean proc; /* If it was processed*/
};
/* MCAP finite state machine functions */
@@ -1573,7 +1574,7 @@ static void mcap_connect_mdl_cb(GIOChannel *chan, GError *conn_err,
mcap_mdl_operation_cb cb = con->cb.op;
gpointer user_data = con->user_data;
- g_free(con);
+ con->proc = TRUE;
DBG("mdl connect callback");
if (conn_err) {
@@ -1592,6 +1593,26 @@ static void mcap_connect_mdl_cb(GIOChannel *chan, GError *conn_err,
cb(mdl, conn_err, user_data);
}
+static void mdl_io_destroy(gpointer data)
+{
+ struct mcap_mdl_op_cb *con = data;
+ struct mcap_mdl *mdl = con->mdl;
+ mcap_mdl_operation_cb cb = con->cb.op;
+ gpointer user_data = con->user_data;
+ gboolean proc = con->proc;
+ GError *err = NULL;
+
+ g_free(con);
+
+ if (proc)
+ return;
+ g_set_error(&err, MCAP_ERROR, MCAP_ERROR_FAILED, "Connection error");
+ mdl->state = MDL_CLOSED;
+ g_io_channel_unref(mdl->dc);
+ mdl->dc = NULL;
+ cb(mdl, err, user_data);
+}
+
gboolean mcap_connect_mdl(struct mcap_mdl *mdl, BtIOType BtType,
uint16_t dcpsm,
mcap_mdl_operation_cb connect_cb,
@@ -1613,7 +1634,7 @@ gboolean mcap_connect_mdl(struct mcap_mdl *mdl, BtIOType BtType,
/* TODO: Check if BtIOType is ERTM or Streaming before continue */
mdl->dc = bt_io_connect(BtType, mcap_connect_mdl_cb, con,
- NULL, err,
+ mdl_io_destroy, err,
BT_IO_OPT_SOURCE_BDADDR, &mdl->mcl->ms->src,
BT_IO_OPT_DEST_BDADDR, &mdl->mcl->addr,
BT_IO_OPT_PSM, dcpsm,
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/2] Avoid possible memory leak in mcap_create_mcl function
From: Santiago Carot-Nemesio @ 2010-09-06 8:15 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
In-Reply-To: <1283760704-5390-1-git-send-email-santoscadenas@gmail.com>
---
health/mcap.c | 28 +++++++++++++++++++++++++---
1 files changed, 25 insertions(+), 3 deletions(-)
diff --git a/health/mcap.c b/health/mcap.c
index 30a1e6e..17e16d8 100644
--- a/health/mcap.c
+++ b/health/mcap.c
@@ -53,6 +53,7 @@ struct connect_mcl {
struct mcap_mcl *mcl; /* MCL for this operation */
mcap_mcl_connect_cb connect_cb; /* Connect callback */
gpointer user_data; /* Callback user data */
+ gboolean proc; /* If it is was proccessed */
};
typedef union {
@@ -1695,8 +1696,7 @@ static void mcap_connect_mcl_cb(GIOChannel *chan, GError *conn_err,
gpointer data = con->user_data;
GError *gerr = NULL;
- g_free(con);
-
+ con->proc = TRUE;
mcl->ctrl &= ~MCAP_CTRL_CONN;
if (conn_err) {
@@ -1760,6 +1760,28 @@ static void connect_dc_event_cb(GIOChannel *chan, GError *err,
mcl->cb->mdl_connected(mdl, mcl->cb->user_data);
}
+static void mcl_io_destroy(gpointer data)
+{
+ struct connect_mcl *con = data;
+ struct mcap_mcl *mcl = con->mcl;
+ mcap_mcl_connect_cb connect_cb = con->connect_cb;
+ gpointer user_data = con->user_data;
+ GError *err = NULL;
+
+ g_free(con);
+
+ if (con->proc)
+ return;
+
+ mcl->ctrl &= ~MCAP_CTRL_CONN;
+
+ if (mcl->ctrl & MCAP_CTRL_FREE)
+ mcl->ms->mcl_uncached_cb(mcl, mcl->ms->user_data);
+ mcap_mcl_check_del(mcl);
+ g_set_error(&err, MCAP_ERROR, MCAP_ERROR_FAILED, "Connection error");
+ connect_cb(NULL, err, user_data);
+}
+
gboolean mcap_create_mcl(struct mcap_instance *ms,
const bdaddr_t *addr,
uint16_t ccpsm,
@@ -1795,7 +1817,7 @@ gboolean mcap_create_mcl(struct mcap_instance *ms,
con->user_data = user_data;
mcl->cc = bt_io_connect(BT_IO_L2CAP, mcap_connect_mcl_cb, con,
- NULL, err,
+ mcl_io_destroy, err,
BT_IO_OPT_SOURCE_BDADDR, &ms->src,
BT_IO_OPT_DEST_BDADDR, addr,
BT_IO_OPT_PSM, ccpsm,
--
1.7.0.4
^ permalink raw reply related
* [PATCH] Check source_id before to call to g_source_remove function
From: Santiago Carot-Nemesio @ 2010-09-06 8:19 UTC (permalink / raw)
To: linux-bluetooth; +Cc: Santiago Carot-Nemesio
---
health/mcap.c | 23 +++++++++++++++--------
1 files changed, 15 insertions(+), 8 deletions(-)
diff --git a/health/mcap.c b/health/mcap.c
index 17e16d8..7dc06ba 100644
--- a/health/mcap.c
+++ b/health/mcap.c
@@ -44,9 +44,11 @@
#define MCAP_ERROR g_quark_from_static_string("mcap-error-quark")
-#define RELEASE_TIMER(__mcl) do { \
- g_source_remove(__mcl->tid); \
- __mcl->tid = 0; \
+#define RELEASE_TIMER(__mcl) do { \
+ if (__mcl->tid) { \
+ g_source_remove(__mcl->tid); \
+ __mcl->tid = 0; \
+ } \
} while(0)
struct connect_mcl {
@@ -227,7 +229,10 @@ static void shutdown_mdl(struct mcap_mdl *mdl)
{
mdl->state = MDL_CLOSED;
- g_source_remove(mdl->wid);
+ if (mdl->wid) {
+ g_source_remove(mdl->wid);
+ mdl->wid = 0;
+ }
if (mdl->dc) {
g_io_channel_shutdown(mdl->dc, TRUE, NULL);
@@ -678,9 +683,7 @@ static void close_mcl(struct mcap_mcl *mcl, gboolean cache_requested)
{
gboolean save = ((!(mcl->ctrl & MCAP_CTRL_FREE)) && cache_requested);
- if (mcl->tid) {
- RELEASE_TIMER(mcl);
- }
+ RELEASE_TIMER(mcl);
if (mcl->cc) {
g_io_channel_shutdown(mcl->cc, TRUE, NULL);
@@ -688,7 +691,11 @@ static void close_mcl(struct mcap_mcl *mcl, gboolean cache_requested)
mcl->cc = NULL;
}
- g_source_remove(mcl->wid);
+ if (mcl->wid) {
+ g_source_remove(mcl->wid);
+ mcl->wid = 0;
+ }
+
if (mcl->lcmd) {
g_free(mcl->lcmd);
mcl->lcmd = NULL;
--
1.7.0.4
^ permalink raw reply related
* Running "putkey" for keyboards and mice
From: Bastien Nocera @ 2010-09-06 10:06 UTC (permalink / raw)
To: BlueZ development
Heya,
Did a small test this week-end on my Macbook, and running "hciconfig"
with putkey works, adding the just paired keyboards' linkkey into the
Bluetooth adapter.
This meant that even though I paired the device in Linux, it was still
available in MacOS X, without any more pairing.
My question is whether we should, for all the adapters listed in the
hid2hci rules, copy the linkkeys for keyboards and (paired) mice[1] to
the adapter itself.
How would you like to see this implemented? Would a device plugin be
good enough to track newly paired devices?
Cheers
[1]: Just the mice that would require pairing, obviously, such as the
Apple mice
^ permalink raw reply
* Re: BD address changed but pairing failed
From: Sebastien Cayetanot @ 2010-09-06 10:17 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=9HR+iTm=baVdefQcd-Nb8Gchx3LN6q-CycMcq@mail.gmail.com>
Le 9/4/2010 9:07 PM, Luiz Augusto von Dentz a écrit :
> Hi,
>
> On Fri, Sep 3, 2010 at 1:30 PM, Sebastien Cayetanot
> <sebastien.cayetanot@linux.intel.com> wrote:
>> All,
>>
>> I'm currently facing an issue regarding the pairing of BT devices after
>> having change the BD_Addr.
>>
>>
>> I use a TI chip and also use hci command or bdaddr script to modify the
>> bd_addr
>>
>> I'm also using the bluez-4.69
>>
>> I change the BD_Addr using the bdaddr scripts provided into the bluez src.
>>
>> When other devices scan me, the new address is seen but hciconfig still show
>> the old BD address.
>>
>> I have sent the hcitool cmd to read the bd_addr and after the hciconfig
>> return the one.
>>
>> I'm discoverable with the new BD address.
>>
>> But When I try to pair an A2DP headset or BT Keyboard/Mouse it failed since
>> BD_Addr change.
>>
>> I have try to use directly the hcitool cmd using the vendor specific command
>> but I got the same result.
>>
>> I have tried to kill bluetooth daemon and restart it after having change the
>> bd_addr. Still failing.
>>
>> I have tried to kill bluetooth daemon before changing the BD adress and
>> start it after the change it still fail.
>>
>> I have also tried to power on/off interface. still the issue
>>
>> I think that something is "broken" is bluez when the BD_Addr is changed.
>>
>> Is someone already seen this issue ?
> I don't think you are suppose to change the address like that, it is
> supposed to be done once in the chip initialization and then keep it
> like that forever, so I guess there is something wrong if the chip is
> already initialize with the wrong address. What are you trying achieve
> with this?
>
Surely not but regarding some tests scripts I have seen on bluez, I
suppose that's possible. Perhaps, some chips support and other don't.
I have tried to change the BD_Address during the BT initialization and
prior any HCI command sent to the chip and it works.
I expected the same behavior than observed on WLAN : I have changed the
WLAN MAC address after and everything works fine.
Regards.
Sebastien
^ permalink raw reply
* [PATCH 1/4] Do not automatically remove watches for service names
From: Luiz Augusto von Dentz @ 2010-09-06 10:26 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto Von Dentz <luiz.dentz-von@nokia.com>
Services can be owned again so it is perfectly fine to keep the watch.
---
gdbus/watch.c | 13 +++++++------
1 files changed, 7 insertions(+), 6 deletions(-)
diff --git a/gdbus/watch.c b/gdbus/watch.c
index 29f23e2..b686c85 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -386,18 +386,19 @@ static DBusHandlerResult service_filter(DBusConnection *connection,
cb->conn_func(connection, cb->user_data);
}
+ /* Only auto remove if it is a bus name watch */
+ if (data->argument[0] == ':' &&
+ (!cb->conn_func || !cb->disc_func)) {
+ filter_data_remove_callback(data, cb);
+ continue;
+ }
+
/* Check if the watch was removed/freed by the callback
* function */
if (!g_slist_find(data->callbacks, cb))
continue;
data->callbacks = g_slist_remove(data->callbacks, cb);
-
- if (!cb->conn_func || !cb->disc_func) {
- g_free(cb);
- continue;
- }
-
data->processed = g_slist_append(data->processed, cb);
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 2/4] Fix signal watch when a service name is given
From: Luiz Augusto von Dentz @ 2010-09-06 10:26 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1283768782-4283-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto Von Dentz <luiz.dentz-von@nokia.com>
The bus name should be resolved when adding a watch by service name since
messages do always come with sender set to owner's bus name, also it
should listen to owner updates since it can change without invalidating
the watch.
---
gdbus/watch.c | 161 +++++++++++++++++++++++++++++++++++++++++++++------------
1 files changed, 128 insertions(+), 33 deletions(-)
diff --git a/gdbus/watch.c b/gdbus/watch.c
index b686c85..8ad4815 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -55,19 +55,22 @@ struct filter_callback {
struct filter_data {
DBusConnection *connection;
DBusHandleMessageFunction handle_func;
- char *sender;
+ char *name;
+ char *owner;
char *path;
char *interface;
char *member;
char *argument;
GSList *callbacks;
GSList *processed;
+ guint name_watch;
gboolean lock;
gboolean registered;
};
static struct filter_data *filter_data_find(DBusConnection *connection,
- const char *sender,
+ const char *name,
+ const char *owner,
const char *path,
const char *interface,
const char *member,
@@ -82,8 +85,12 @@ static struct filter_data *filter_data_find(DBusConnection *connection,
if (connection != data->connection)
continue;
- if (sender && data->sender &&
- g_str_equal(sender, data->sender) == FALSE)
+ if (name && data->name &&
+ g_str_equal(name, data->name) == FALSE)
+ continue;
+
+ if (owner && data->owner &&
+ g_str_equal(owner, data->owner) == FALSE)
continue;
if (path && data->path &&
@@ -110,13 +117,15 @@ static struct filter_data *filter_data_find(DBusConnection *connection,
static void format_rule(struct filter_data *data, char *rule, size_t size)
{
+ const char *sender;
int offset;
offset = snprintf(rule, size, "type='signal'");
+ sender = data->name ? : data->owner;
- if (data->sender)
+ if (sender)
offset += snprintf(rule + offset, size - offset,
- ",sender='%s'", data->sender);
+ ",sender='%s'", sender);
if (data->path)
offset += snprintf(rule + offset, size - offset,
",path='%s'", data->path);
@@ -183,8 +192,10 @@ static struct filter_data *filter_data_get(DBusConnection *connection,
const char *argument)
{
struct filter_data *data;
+ const char *name = NULL, *owner = NULL;
- if (!filter_data_find(connection, NULL, NULL, NULL, NULL, NULL)) {
+ if (!filter_data_find(connection, NULL, NULL, NULL, NULL, NULL,
+ NULL)) {
if (!dbus_connection_add_filter(connection,
message_filter, NULL, NULL)) {
error("dbus_connection_add_filter() failed");
@@ -192,15 +203,25 @@ static struct filter_data *filter_data_get(DBusConnection *connection,
}
}
- data = filter_data_find(connection, sender, path, interface, member,
- argument);
+ if (sender == NULL)
+ goto proceed;
+
+ if (sender[0] == ':')
+ owner = sender;
+ else
+ name = sender;
+
+proceed:
+ data = filter_data_find(connection, name, owner, path, interface,
+ member, argument);
if (data)
return data;
data = g_new0(struct filter_data, 1);
data->connection = dbus_connection_ref(connection);
- data->sender = g_strdup(sender);
+ data->name = name ? g_strdup(name) : NULL;
+ data->owner = owner ? g_strdup(owner) : NULL;
data->path = g_strdup(path);
data->interface = g_strdup(interface);
data->member = g_strdup(member);
@@ -244,7 +265,9 @@ static void filter_data_free(struct filter_data *data)
g_free(l->data);
g_slist_free(data->callbacks);
- g_free(data->sender);
+ g_dbus_remove_watch(data->connection, data->name_watch);
+ g_free(data->name);
+ g_free(data->owner);
g_free(data->path);
g_free(data->interface);
g_free(data->member);
@@ -322,7 +345,8 @@ static gboolean filter_data_remove_callback(struct filter_data *data,
filter_data_free(data);
/* Remove filter if there are no listeners left for the connection */
- data = filter_data_find(connection, NULL, NULL, NULL, NULL, NULL);
+ data = filter_data_find(connection, NULL, NULL, NULL, NULL, NULL,
+ NULL);
if (!data)
dbus_connection_remove_filter(connection, message_filter,
NULL);
@@ -359,6 +383,37 @@ static DBusHandlerResult signal_filter(DBusConnection *connection,
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
}
+static void update_name_cache(const char *name, const char *owner)
+{
+ GSList *l;
+
+ for (l = listeners; l != NULL; l = l->next) {
+ struct filter_data *data = l->data;
+
+ if (g_strcmp0(data->name, name) != 0)
+ continue;
+
+ g_free(data->owner);
+ data->owner = g_strdup(owner);
+ }
+}
+
+static const char *check_name_cache(const char *name)
+{
+ GSList *l;
+
+ for (l = listeners; l != NULL; l = l->next) {
+ struct filter_data *data = l->data;
+
+ if (g_strcmp0(data->name, name) != 0)
+ continue;
+
+ return data->owner;
+ }
+
+ return NULL;
+}
+
static DBusHandlerResult service_filter(DBusConnection *connection,
DBusMessage *message, void *user_data)
{
@@ -375,6 +430,8 @@ static DBusHandlerResult service_filter(DBusConnection *connection,
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
}
+ update_name_cache(name, new);
+
while (data->callbacks) {
cb = data->callbacks->data;
@@ -422,7 +479,9 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
member = dbus_message_get_member(message);
dbus_message_get_args(message, NULL, DBUS_TYPE_STRING, &arg, DBUS_TYPE_INVALID);
- data = filter_data_find(connection, sender, path, iface, member, arg);
+ /* Sender is always bus name */
+ data = filter_data_find(connection, NULL, sender, path, iface, member,
+ arg);
if (!data) {
error("Got %s.%s signal which has no listeners", iface, member);
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
@@ -447,7 +506,8 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
filter_data_free(data);
/* Remove filter if there no listener left for the connection */
- data = filter_data_find(connection, NULL, NULL, NULL, NULL, NULL);
+ data = filter_data_find(connection, NULL, NULL, NULL, NULL, NULL,
+ NULL);
if (!data)
dbus_connection_remove_filter(connection, message_filter,
NULL);
@@ -457,38 +517,60 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
struct service_data {
DBusConnection *conn;
+ char *name;
+ const char *owner;
GDBusWatchFunction conn_func;
void *user_data;
};
+static void service_data_free(struct service_data *data)
+{
+ dbus_connection_unref(data->conn);
+ g_free(data->name);
+ g_free(data);
+}
+
+static gboolean update_service(void *user_data)
+{
+ struct service_data *data = user_data;
+
+ update_name_cache(data->name, data->owner);
+ if (data->conn_func)
+ data->conn_func(data->conn, data->user_data);
+
+ service_data_free(data);
+
+ return FALSE;
+}
+
static void service_reply(DBusPendingCall *call, void *user_data)
{
struct service_data *data = user_data;
DBusMessage *reply;
- DBusError error;
- dbus_bool_t has_owner;
+ DBusError err;
reply = dbus_pending_call_steal_reply(call);
if (reply == NULL)
return;
- dbus_error_init(&error);
+ dbus_error_init(&err);
- if (dbus_message_get_args(reply, &error,
- DBUS_TYPE_BOOLEAN, &has_owner,
- DBUS_TYPE_INVALID) == FALSE) {
- if (dbus_error_is_set(&error) == TRUE) {
- error("%s", error.message);
- dbus_error_free(&error);
- } else {
- error("Wrong arguments for NameHasOwner reply");
- }
- goto done;
- }
+ if (dbus_set_error_from_message(&err, reply))
+ goto fail;
- if (has_owner && data->conn_func)
- data->conn_func(data->conn, data->user_data);
+ if (dbus_message_get_args(reply, &err,
+ DBUS_TYPE_STRING, &data->owner,
+ DBUS_TYPE_INVALID) == FALSE)
+ goto fail;
+ update_service(data);
+
+ goto done;
+
+fail:
+ error("%s", err.message);
+ dbus_error_free(&err);
+ service_data_free(data);
done:
dbus_message_unref(reply);
}
@@ -506,12 +588,19 @@ static void check_service(DBusConnection *connection, const char *name,
return;
}
- data->conn = connection;
+ data->conn = dbus_connection_ref(connection);
+ data->name = g_strdup(name);
data->conn_func = connect;
data->user_data = user_data;
+ data->owner = check_name_cache(name);
+ if (data->owner != NULL) {
+ g_idle_add(update_service, data);
+ return;
+ }
+
message = dbus_message_new_method_call(DBUS_SERVICE_DBUS,
- DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, "NameHasOwner");
+ DBUS_PATH_DBUS, DBUS_INTERFACE_DBUS, "GetNameOwner");
if (message == NULL) {
error("Can't allocate new message");
g_free(data);
@@ -597,6 +686,11 @@ guint g_dbus_add_signal_watch(DBusConnection *connection,
if (!cb)
return 0;
+ if (data->name != NULL && data->name_watch == 0)
+ data->name_watch = g_dbus_add_service_watch(connection,
+ data->name, NULL,
+ NULL, NULL, NULL);
+
return cb->id;
}
@@ -626,7 +720,8 @@ void g_dbus_remove_all_watches(DBusConnection *connection)
{
struct filter_data *data;
- while ((data = filter_data_find(connection, NULL, NULL, NULL, NULL, NULL))) {
+ while ((data = filter_data_find(connection, NULL, NULL, NULL, NULL,
+ NULL, NULL))) {
listeners = g_slist_remove(listeners, data);
filter_data_call_and_free(data);
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 3/4] Fix calling watch callbacks after it has been removed
From: Luiz Augusto von Dentz @ 2010-09-06 10:26 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1283768782-4283-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto Von Dentz <luiz.dentz-von@nokia.com>
Pending call should be removed if the watch is removed since the
application no longer expect that to be reached and may already freed the
data associated with it.
---
gdbus/watch.c | 75 ++++++++++++++++++++++++++++++++++++--------------------
1 files changed, 48 insertions(+), 27 deletions(-)
diff --git a/gdbus/watch.c b/gdbus/watch.c
index 8ad4815..249d927 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -43,11 +43,21 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
static guint listener_id = 0;
static GSList *listeners = NULL;
+struct service_data {
+ DBusConnection *conn;
+ DBusPendingCall *call;
+ char *name;
+ const char *owner;
+ guint id;
+ struct filter_callback *callback;
+};
+
struct filter_callback {
GDBusWatchFunction conn_func;
GDBusWatchFunction disc_func;
GDBusSignalFunction signal_func;
GDBusDestroyFunction destroy_func;
+ struct service_data *data;
void *user_data;
guint id;
};
@@ -302,7 +312,7 @@ static struct filter_callback *filter_data_add_callback(
{
struct filter_callback *cb = NULL;
- cb = g_new(struct filter_callback, 1);
+ cb = g_new0(struct filter_callback, 1);
cb->conn_func = connect;
cb->disc_func = disconnect;
@@ -319,6 +329,24 @@ static struct filter_callback *filter_data_add_callback(
return cb;
}
+static void service_data_free(struct service_data *data)
+{
+ struct filter_callback *callback = data->callback;
+
+ dbus_connection_unref(data->conn);
+
+ if (data->call)
+ dbus_pending_call_unref(data->call);
+
+ if (data->id)
+ g_source_remove(data->id);
+
+ g_free(data->name);
+ g_free(data);
+
+ callback->data = NULL;
+}
+
static gboolean filter_data_remove_callback(struct filter_data *data,
struct filter_callback *cb)
{
@@ -327,6 +355,13 @@ static gboolean filter_data_remove_callback(struct filter_data *data,
data->callbacks = g_slist_remove(data->callbacks, cb);
data->processed = g_slist_remove(data->processed, cb);
+ /* Cancel pending operations */
+ if (cb->data) {
+ if (cb->data->call)
+ dbus_pending_call_cancel(cb->data->call);
+ service_data_free(cb->data);
+ }
+
if (cb->destroy_func)
cb->destroy_func(cb->user_data);
@@ -515,28 +550,14 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
}
-struct service_data {
- DBusConnection *conn;
- char *name;
- const char *owner;
- GDBusWatchFunction conn_func;
- void *user_data;
-};
-
-static void service_data_free(struct service_data *data)
-{
- dbus_connection_unref(data->conn);
- g_free(data->name);
- g_free(data);
-}
-
static gboolean update_service(void *user_data)
{
struct service_data *data = user_data;
+ struct filter_callback *cb = data->callback;
update_name_cache(data->name, data->owner);
- if (data->conn_func)
- data->conn_func(data->conn, data->user_data);
+ if (cb->conn_func)
+ cb->conn_func(data->conn, cb->user_data);
service_data_free(data);
@@ -575,11 +596,11 @@ done:
dbus_message_unref(reply);
}
-static void check_service(DBusConnection *connection, const char *name,
- GDBusWatchFunction connect, void *user_data)
+static void check_service(DBusConnection *connection,
+ const char *name,
+ struct filter_callback *callback)
{
DBusMessage *message;
- DBusPendingCall *call;
struct service_data *data;
data = g_try_malloc0(sizeof(*data));
@@ -590,12 +611,12 @@ static void check_service(DBusConnection *connection, const char *name,
data->conn = dbus_connection_ref(connection);
data->name = g_strdup(name);
- data->conn_func = connect;
- data->user_data = user_data;
+ data->callback = callback;
+ callback->data = data;
data->owner = check_name_cache(name);
if (data->owner != NULL) {
- g_idle_add(update_service, data);
+ data->id = g_idle_add(update_service, data);
return;
}
@@ -611,13 +632,13 @@ static void check_service(DBusConnection *connection, const char *name,
DBUS_TYPE_INVALID);
if (dbus_connection_send_with_reply(connection, message,
- &call, -1) == FALSE) {
+ &data->call, -1) == FALSE) {
error("Failed to execute method call");
g_free(data);
goto done;
}
- if (call == NULL) {
+ if (data->call == NULL) {
error("D-Bus connection not available");
g_free(data);
goto done;
@@ -654,7 +675,7 @@ guint g_dbus_add_service_watch(DBusConnection *connection, const char *name,
return 0;
if (connect)
- check_service(connection, name, connect, user_data);
+ check_service(connection, name, cb);
return cb->id;
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH 4/4] Make use of ofono bus name when watching its signals
From: Luiz Augusto von Dentz @ 2010-09-06 10:26 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1283768782-4283-1-git-send-email-luiz.dentz@gmail.com>
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
---
audio/telephony-ofono.c | 6 +++---
1 files changed, 3 insertions(+), 3 deletions(-)
diff --git a/audio/telephony-ofono.c b/audio/telephony-ofono.c
index 9c20112..693207c 100644
--- a/audio/telephony-ofono.c
+++ b/audio/telephony-ofono.c
@@ -1069,13 +1069,13 @@ int telephony_init(void)
connection = dbus_bus_get(DBUS_BUS_SYSTEM, NULL);
- registration_watch = g_dbus_add_signal_watch(connection, NULL, NULL,
- OFONO_NETWORKREG_INTERFACE,
+ registration_watch = g_dbus_add_signal_watch(connection, OFONO_BUS_NAME,
+ NULL, OFONO_NETWORKREG_INTERFACE,
"PropertyChanged",
handle_registration_property_changed,
NULL, NULL);
- voice_watch = g_dbus_add_signal_watch(connection, NULL, NULL,
+ voice_watch = g_dbus_add_signal_watch(connection, OFONO_BUS_NAME, NULL,
OFONO_VCMANAGER_INTERFACE,
"PropertyChanged",
handle_vcmanager_property_changed,
--
1.7.0.4
^ permalink raw reply related
* Add new plugin to set adapter class
From: Bastien Nocera @ 2010-09-06 10:48 UTC (permalink / raw)
To: BlueZ development
[-- Attachment #1: Type: text/plain, Size: 109 bytes --]
To replace the old code that was using HAL.
This allows us to avoid HAL usage, as it is deprecated.
Cheers
[-- Attachment #2: 0001-Add-new-plugin-to-set-adapter-class.patch --]
[-- Type: text/x-patch, Size: 6921 bytes --]
>From f8bb02df7e3c13f71105ceb8c116294cb3a0f6c2 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Mon, 6 Sep 2010 11:44:31 +0100
Subject: [PATCH] Add new plugin to set adapter class
This time based on the kernel exported DMI, rather than calling
out to HAL. The HAL plugin can still be preferred with
--enable-hal=yes passed to configure.
---
Makefile.am | 7 ++
acinclude.m4 | 7 ++-
plugins/formfactor.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 164 insertions(+), 1 deletions(-)
create mode 100644 plugins/formfactor.c
diff --git a/Makefile.am b/Makefile.am
index 665c0a9..3fb948f 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -199,8 +199,15 @@ endif
builtin_modules += hciops
builtin_sources += plugins/hciops.c
+if HAL
builtin_modules += hal
builtin_sources += plugins/hal.c
+else
+builtin_modules += formfactor
+builtin_sources += plugins/formfactor.c
+endif
+
+EXTRA_DIST += plugins/hal.c plugins/formfactor.c
builtin_modules += storage
builtin_sources += plugins/storage.c
diff --git a/acinclude.m4 b/acinclude.m4
index 25cd259..2566d0d 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -178,7 +178,7 @@ AC_DEFUN([AC_ARG_BLUEZ], [
capng_enable=${capng_found}
sndfile_enable=${sndfile_found}
netlink_enable=no
- hal_enable=${hal_found}
+ hal_enable=no
usb_enable=${usb_found}
cable_enable=${cable_found}
alsa_enable=${alsa_found}
@@ -337,6 +337,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
maemo6_enable=${enableval}
])
+ AC_ARG_ENABLE(hal, AC_HELP_STRING([--enable-hal], [Use HAL to determine adapter class]), [
+ hal_enable=${enableval}
+ ])
+
if (test "${fortify_enable}" = "yes"); then
CFLAGS="$CFLAGS -D_FORTIFY_SOURCE=2"
fi
@@ -378,6 +382,7 @@ AC_DEFUN([AC_ARG_BLUEZ], [
AM_CONDITIONAL(NETWORKPLUGIN, test "${network_enable}" = "yes")
AM_CONDITIONAL(SERVICEPLUGIN, test "${service_enable}" = "yes")
AM_CONDITIONAL(MCAP, test "${mcap_enable}" = "yes")
+ AM_CONDITIONAL(HAL, test "${hal_enable}" = "yes")
AM_CONDITIONAL(ATTRIBPLUGIN, test "${attrib_enable}" = "yes")
AM_CONDITIONAL(ECHOPLUGIN, test "no" = "yes")
AM_CONDITIONAL(PNATPLUGIN, test "${pnat_enable}" = "yes")
diff --git a/plugins/formfactor.c b/plugins/formfactor.c
new file mode 100644
index 0000000..037a768
--- /dev/null
+++ b/plugins/formfactor.c
@@ -0,0 +1,151 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2004-2010 Marcel Holtmann <marcel@holtmann.org>
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <errno.h>
+
+#include <bluetooth/bluetooth.h>
+#include <bluetooth/hci.h>
+#include <bluetooth/hci_lib.h>
+
+#include "plugin.h"
+#include "adapter.h"
+#include "log.h"
+#include "dbus-hci.h"
+
+#define DMI_CHASSIS_FILE "/sys/class/dmi/id/chassis_type"
+#define DMI_CHASSIS_FILE_FALLBACK "/sys/devices/virtual/dmi/id/chassis_type"
+
+/* Map the chassis type from chassis_type to a sensible type used in hal
+ *
+ * See also 3.3.4.1 of the "System Management BIOS Reference Specification,
+ * Version 2.6.1" (Preliminary Standard) document, available from
+ * http://www.dmtf.org/standards/smbios.
+ *
+ * TODO: figure out WTF the mapping should be; "Lunch Box"? Give me a break :-)
+ *
+ * Copied from hal/hald/linux/osspec.c
+ */
+static const char *chassis_map[] = {
+ "Other", "unknown", /* 0x01 */
+ "Unknown", "unknown",
+ "Desktop", "desktop",
+ "Low Profile Desktop", "desktop",
+ "Pizza Box", "server",
+ "Mini Tower", "desktop",
+ "Tower", "desktop",
+ "Portable", "laptop",
+ "Laptop", "laptop",
+ "Notebook", "laptop",
+ "Hand Held", "handheld",
+ "Docking Station", "laptop",
+ "All In One", "unknown",
+ "Sub Notebook", "laptop",
+ "Space-saving", "desktop",
+ "Lunch Box", "unknown",
+ "Main Server Chassis", "server",
+ "Expansion Chassis", "unknown",
+ "Sub Chassis", "unknown",
+ "Bus Expansion Chassis", "unknown",
+ "Peripheral Chassis", "unknown",
+ "RAID Chassis", "unknown",
+ "Rack Mount Chassis", "unknown",
+ "Sealed-case PC", "unknown",
+ "Multi-system", "unknown",
+ "CompactPCI", "unknonw",
+ "AdvancedTCA", "unknown",
+ "Blade", "server",
+ "Blade Enclosure" "unknown", /* 0x1D */
+ NULL
+};
+
+static int formfactor_probe(struct btd_adapter *adapter)
+{
+ int chassis_type;
+ uint8_t minor = 0;
+ const char *formfactor;
+ char *contents;
+
+ if (g_file_get_contents(DMI_CHASSIS_FILE,
+ &contents, NULL, NULL) == FALSE) {
+ if (g_file_get_contents(DMI_CHASSIS_FILE_FALLBACK,
+ &contents, NULL, NULL) == FALSE) {
+ error("Could not get the contents of DMI chassis type");
+ return 0;
+ }
+ }
+
+ chassis_type = atoi(contents);
+ g_free (contents);
+
+ if (chassis_type > 0x1D || chassis_type <= 0) {
+ error ("Chassis type is not a known chassis type");
+ return 0;
+ }
+
+ formfactor = chassis_map[chassis_type * 2];
+ if (formfactor != NULL) {
+ if (g_str_equal(formfactor, "laptop") == TRUE)
+ minor |= (1 << 2) | (1 << 3);
+ else if (g_str_equal(formfactor, "desktop") == TRUE)
+ minor |= 1 << 2;
+ else if (g_str_equal(formfactor, "server") == TRUE)
+ minor |= 1 << 3;
+ else if (g_str_equal(formfactor, "handheld") == TRUE)
+ minor += 1 << 4;
+ }
+
+ /* Computer major class */
+ DBG("Setting 0x%06x for major/minor device class", (1 << 8) | minor);
+
+ btd_adapter_set_class(adapter, 0x01, minor);
+
+ return 0;
+}
+
+static void formfactor_remove(struct btd_adapter *adapter)
+{
+}
+
+static struct btd_adapter_driver formfactor_driver = {
+ .name = "formfactor",
+ .probe = formfactor_probe,
+ .remove = formfactor_remove,
+};
+
+static int formfactor_init(void)
+{
+ return btd_register_adapter_driver(&formfactor_driver);
+}
+
+static void formfactor_exit(void)
+{
+ btd_unregister_adapter_driver(&formfactor_driver);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(formfactor, VERSION,
+ BLUETOOTH_PLUGIN_PRIORITY_LOW, formfactor_init, formfactor_exit)
--
1.7.0.1
^ permalink raw reply related
* Re: Add new plugin to set adapter class
From: Johan Hedberg @ 2010-09-06 11:15 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ development
In-Reply-To: <1283770089.7529.9.camel@localhost.localdomain>
Hi Bastien,
On Mon, Sep 06, 2010, Bastien Nocera wrote:
> +if HAL
> builtin_modules += hal
> builtin_sources += plugins/hal.c
> +else
> +builtin_modules += formfactor
> +builtin_sources += plugins/formfactor.c
> +endif
Previously it was possible to --disable-hal and be sure that whatever is
defined in main.conf gets used (this is e.g. something that has been
important for us in Maemo/MeeGo). Your patch seems to make one of the
two automated adapter class sources always be used and so potentially
overriding what main.conf says. So I'd prefer to keep the possibility of
disably any such plugins completely.
Johan
^ permalink raw reply
* Re: Add new plugin to set adapter class
From: Bastien Nocera @ 2010-09-06 11:17 UTC (permalink / raw)
To: Johan Hedberg; +Cc: BlueZ development
In-Reply-To: <20100906111516.GA23167@jh-x301>
On Mon, 2010-09-06 at 14:15 +0300, Johan Hedberg wrote:
> Hi Bastien,
>
> On Mon, Sep 06, 2010, Bastien Nocera wrote:
> > +if HAL
> > builtin_modules += hal
> > builtin_sources += plugins/hal.c
> > +else
> > +builtin_modules += formfactor
> > +builtin_sources += plugins/formfactor.c
> > +endif
>
> Previously it was possible to --disable-hal and be sure that whatever is
> defined in main.conf gets used (this is e.g. something that has been
> important for us in Maemo/MeeGo). Your patch seems to make one of the
> two automated adapter class sources always be used and so potentially
> overriding what main.conf says. So I'd prefer to keep the possibility of
> disably any such plugins completely.
Take a look at the code, there's no ways to disable the HAL plugin right
now. If you want a separate patch for that, I can certainly cook one up.
^ permalink raw reply
* Re: Add new plugin to set adapter class
From: Johan Hedberg @ 2010-09-06 11:30 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ development
In-Reply-To: <1283771863.7529.10.camel@localhost.localdomain>
Hi Bastien,
On Mon, Sep 06, 2010, Bastien Nocera wrote:
> > Previously it was possible to --disable-hal and be sure that whatever is
> > defined in main.conf gets used (this is e.g. something that has been
> > important for us in Maemo/MeeGo). Your patch seems to make one of the
> > two automated adapter class sources always be used and so potentially
> > overriding what main.conf says. So I'd prefer to keep the possibility of
> > disably any such plugins completely.
>
> Take a look at the code, there's no ways to disable the HAL plugin right
> now. If you want a separate patch for that, I can certainly cook one up.
Yep, my memory was playing tricks on me here. We're actually using
DisablePlugins=hal in main.conf to avoid interference from this code.
The same would also work for your new plugin so build-time disabling
isn't strictly necessary then (but might be nice to have for some
platforms).
Johan
^ permalink raw reply
* Re: [PATCH] Bluetooth: check L2CAP length in first ACL fragment
From: Andrei Emeltchenko @ 2010-09-06 11:32 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth
In-Reply-To: <alpine.DEB.2.00.1009030837330.26179@linux-sea-02>
Hi,
On Fri, Sep 3, 2010 at 7:26 PM, Mat Martineau <mathewm@codeaurora.org> wrote:
>> Trace below is received when using stress tools sending big
>> fragmented L2CAP packets.
>> ...
>> [ 1712.798492] swapper: page allocation failure. order:4, mode:0x4020
>> [ 1712.804809] [<c0031870>] (unwind_backtrace+0x0/0xdc) from [<c00a1f70>]
>> (__alloc_pages_nodemask+0x4)
>> [ 1712.814666] [<c00a1f70>] (__alloc_pages_nodemask+0x47c/0x4d4) from
>> [<c00a1fd8>] (__get_free_pages+)
>> [ 1712.824645] [<c00a1fd8>] (__get_free_pages+0x10/0x3c) from [<c026eb5c>]
>> (__alloc_skb+0x4c/0xfc)
>> [ 1712.833465] [<c026eb5c>] (__alloc_skb+0x4c/0xfc) from [<bf28c738>]
>> (l2cap_recv_acldata+0xf0/0x1f8 )
>> [ 1712.843322] [<bf28c738>] (l2cap_recv_acldata+0xf0/0x1f8 [l2cap]) from
>> [<bf0094ac>] (hci_rx_task+0x)
>> ...
>
> What is logged right before this allocation failure? There should be a
> "Start: total len %d, frag len %d" line. Actually, all log messages from
> l2cap_recv_acldata leading up to the failure would be helpful.
When I enable logs system behave differently because of huge traffic
to syslog :-)
>> After applying patch dmesg looks like:
>> ...
>> l2cap_recv_acldata: Frame exceeding recv MTU (len 64182, MTU 1013)
>> l2cap_recv_acldata: Unexpected continuation frame (len 34)
>> ...
>>
>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>> ---
>> net/bluetooth/l2cap.c | 11 +++++++++++
>> 1 files changed, 11 insertions(+), 0 deletions(-)
>>
>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>> index 9fad312..21824d7 100644
>> --- a/net/bluetooth/l2cap.c
>> +++ b/net/bluetooth/l2cap.c
>> @@ -4654,6 +4654,8 @@ static int l2cap_recv_acldata(struct hci_conn *hcon,
>> struct sk_buff *skb, u16 fl
>>
>> if (flags & ACL_START) {
>> struct l2cap_hdr *hdr;
>> + struct sock *sk;
>> + u16 cid;
>> int len;
>>
>> if (conn->rx_len) {
>> @@ -4672,6 +4674,7 @@ static int l2cap_recv_acldata(struct hci_conn *hcon,
>> struct sk_buff *skb, u16 fl
>> d
>> hdr = (struct l2cap_hdr *) skb->data;
>> len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE;
>> + cid = __le16_to_cpu(hdr->cid);
>
> Some stress tools also send L2CAP data one byte at a time - the start
> fragment may not have all four header bytes. l2cap_recv_acldata() currently
> allows short start fragments with two or three bytes, which would not
> contain a valid CID.
Yes currently we allow 2 bytes in start fragment and check hdr->len.
What about following change (require 4 bytes in start fragment)
---------------------------------------------
- if (skb->len < 2) {
+ if (skb->len < L2CAP_HDR_SIZE) {
BT_ERR("Frame is too short (len %d)", skb->len);
l2cap_conn_unreliable(conn, ECOMM);
goto drop;
---------------------------------------------
>> if (len == skb->len) {
>> /* Complete frame received */
>> @@ -4688,6 +4691,14 @@ static int l2cap_recv_acldata(struct hci_conn
>> *hcon, struct sk_buff *skb, u16 fl
>> goto drop;
>> }
>>
>> + sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
>
> There are several issues here.
>
> l2cap_get_chan_by_scid() might return NULL, which will definitely happen
> with signaling or connectionless data frames.
>
> l2cap_get_chan_by_scid() also calls bh_lock_sock() if a channel is found,
> and I don't see that you added a matching call to bh_unlock_sock() if
> l2cap_get_chan_by_scid() returns a non-NULL value.
My mistake here. What about following check:
---------------------------------------
@@ -3916,6 +3919,19 @@ static int l2cap_recv_acldata(struct hci_conn
*hcon, struct sk_buff *skb,
goto drop;
}
+ sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
+ if (!sk)
+ goto drop;
+
+ if (l2cap_pi(sk)->imtu < len) {
+ BT_ERR("Frame exceeding recv MTU (len %d, MTU %d)",
+ len, l2cap_pi(sk)->imtu);
+ conn->rx_len = 0; /* needed? */
+ bh_unlock_sock(sk);
+ goto drop;
+ } else
+ bh_unlock_sock(sk);
+
/* Allocate skb for the complete frame (with header) */
conn->rx_skb = bt_skb_alloc(len, GFP_ATOMIC);
if (!conn->rx_skb)
--------------------------------------
>
>> + if (l2cap_pi(sk)->imtu < len) {
>> + BT_ERR("Frame exceeding recv MTU (len %d, MTU
>> %d)",
>> + len, l2cap_pi(sk)->imtu);
>> + conn->rx_len = 0; /* needed? */
>> + goto drop;
>> + }
>> +
>> /* Allocate skb for the complete frame (with header) */
>> conn->rx_skb = bt_skb_alloc(len, GFP_ATOMIC);
>> if (!conn->rx_skb)
>
> In general, this approach adds an extra channel lookup and an extra
> lock/unlock on every ACL start frame.
Yes this adds extra lock/unlock but only if the frame is not complete. There
shouldn't be many fragmented L2CAP packets.
> Even if the MTU is known to be
> exceeded on with the start frame, no more than 64k is ever allocated (which
> shouldn't cause problems in itself).
I think that for the mobile device this can be a problem since this
shall be contiguous
memory.
> The root of the problem may be heap
> corruption due to a buffer overrun, which seems possible due to the crash
> deep in the memory allocator.
Regards,
Andrei
^ permalink raw reply
* Re: Running "putkey" for keyboards and mice
From: Luiz Augusto von Dentz @ 2010-09-06 11:51 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ development
In-Reply-To: <1283767613.7529.6.camel@localhost.localdomain>
Hi,
On Mon, Sep 6, 2010 at 1:06 PM, Bastien Nocera <hadess@hadess.net> wrote:
> Heya,
>
> Did a small test this week-end on my Macbook, and running "hciconfig"
> with putkey works, adding the just paired keyboards' linkkey into the
> Bluetooth adapter.
>
> This meant that even though I paired the device in Linux, it was still
> available in MacOS X, without any more pairing.
>
> My question is whether we should, for all the adapters listed in the
> hid2hci rules, copy the linkkeys for keyboards and (paired) mice[1] to
> the adapter itself.
>
> How would you like to see this implemented? Would a device plugin be
> good enough to track newly paired devices?
Im not sure this is going to work the other way round, when you pair
in some other SO and than save the key in the chip we would need to
load this device as it is already paired. If that can be done than
IMOH this should be integrated directly in core, perhaps adding
.putkey/.delkey/.getkeys to hciops so we can properly use the keys
stored on chip, but I guess we need to maintain our own storage too
since chips normally have very limited space for storing keys.
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* Re: Add new plugin to set adapter class
From: Bastien Nocera @ 2010-09-06 12:05 UTC (permalink / raw)
To: Johan Hedberg; +Cc: BlueZ development
In-Reply-To: <20100906113031.GA23586@jh-x301>
[-- Attachment #1: Type: text/plain, Size: 1115 bytes --]
On Mon, 2010-09-06 at 14:30 +0300, Johan Hedberg wrote:
> Hi Bastien,
>
> On Mon, Sep 06, 2010, Bastien Nocera wrote:
> > > Previously it was possible to --disable-hal and be sure that whatever is
> > > defined in main.conf gets used (this is e.g. something that has been
> > > important for us in Maemo/MeeGo). Your patch seems to make one of the
> > > two automated adapter class sources always be used and so potentially
> > > overriding what main.conf says. So I'd prefer to keep the possibility of
> > > disably any such plugins completely.
> >
> > Take a look at the code, there's no ways to disable the HAL plugin right
> > now. If you want a separate patch for that, I can certainly cook one up.
>
> Yep, my memory was playing tricks on me here. We're actually using
> DisablePlugins=hal in main.conf to avoid interference from this code.
> The same would also work for your new plugin so build-time disabling
> isn't strictly necessary then (but might be nice to have for some
> platforms).
OK. Patch that actually applies to master attached. Was conflicting with
another non-upstreamed patch.
Cheers
[-- Attachment #2: 0001-Add-new-plugin-to-set-adapter-class.patch --]
[-- Type: text/x-patch, Size: 6917 bytes --]
>From 6921ff03adcaea6e10dd926b8c9e3b9ec8842334 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Mon, 6 Sep 2010 11:44:31 +0100
Subject: [PATCH] Add new plugin to set adapter class
This time based on the kernel exported DMI, rather than calling
out to HAL. The HAL plugin can still be preferred with
--enable-hal=yes passed to configure.
---
Makefile.am | 7 ++
acinclude.m4 | 7 ++-
plugins/formfactor.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++
3 files changed, 164 insertions(+), 1 deletions(-)
create mode 100644 plugins/formfactor.c
diff --git a/Makefile.am b/Makefile.am
index 9043d25..9a78780 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -194,8 +194,15 @@ endif
builtin_modules += hciops
builtin_sources += plugins/hciops.c
+if HAL
builtin_modules += hal
builtin_sources += plugins/hal.c
+else
+builtin_modules += formfactor
+builtin_sources += plugins/formfactor.c
+endif
+
+EXTRA_DIST += plugins/hal.c plugins/formfactor.c
builtin_modules += storage
builtin_sources += plugins/storage.c
diff --git a/acinclude.m4 b/acinclude.m4
index dda965e..b34f08d 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -158,7 +158,7 @@ AC_DEFUN([AC_ARG_BLUEZ], [
fortify_enable=yes
pie_enable=yes
sndfile_enable=${sndfile_found}
- hal_enable=${hal_found}
+ hal_enable=no
usb_enable=${usb_found}
alsa_enable=${alsa_found}
gstreamer_enable=${gstreamer_found}
@@ -308,6 +308,10 @@ AC_DEFUN([AC_ARG_BLUEZ], [
maemo6_enable=${enableval}
])
+ AC_ARG_ENABLE(hal, AC_HELP_STRING([--enable-hal], [Use HAL to determine adapter class]), [
+ hal_enable=${enableval}
+ ])
+
if (test "${fortify_enable}" = "yes"); then
CFLAGS="$CFLAGS -D_FORTIFY_SOURCE=2"
fi
@@ -341,6 +345,7 @@ AC_DEFUN([AC_ARG_BLUEZ], [
AM_CONDITIONAL(NETWORKPLUGIN, test "${network_enable}" = "yes")
AM_CONDITIONAL(SERVICEPLUGIN, test "${service_enable}" = "yes")
AM_CONDITIONAL(MCAP, test "${mcap_enable}" = "yes")
+ AM_CONDITIONAL(HAL, test "${hal_enable}" = "yes")
AM_CONDITIONAL(ATTRIBPLUGIN, test "${attrib_enable}" = "yes")
AM_CONDITIONAL(ECHOPLUGIN, test "no" = "yes")
AM_CONDITIONAL(PNATPLUGIN, test "${pnat_enable}" = "yes")
diff --git a/plugins/formfactor.c b/plugins/formfactor.c
new file mode 100644
index 0000000..037a768
--- /dev/null
+++ b/plugins/formfactor.c
@@ -0,0 +1,151 @@
+/*
+ *
+ * BlueZ - Bluetooth protocol stack for Linux
+ *
+ * Copyright (C) 2004-2010 Marcel Holtmann <marcel@holtmann.org>
+ *
+ *
+ * This program is free software; you can redistribute it and/or modify
+ * it under the terms of the GNU General Public License as published by
+ * the Free Software Foundation; either version 2 of the License, or
+ * (at your option) any later version.
+ *
+ * This program is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
+ * GNU General Public License for more details.
+ *
+ * You should have received a copy of the GNU General Public License
+ * along with this program; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301 USA
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <errno.h>
+
+#include <bluetooth/bluetooth.h>
+#include <bluetooth/hci.h>
+#include <bluetooth/hci_lib.h>
+
+#include "plugin.h"
+#include "adapter.h"
+#include "log.h"
+#include "dbus-hci.h"
+
+#define DMI_CHASSIS_FILE "/sys/class/dmi/id/chassis_type"
+#define DMI_CHASSIS_FILE_FALLBACK "/sys/devices/virtual/dmi/id/chassis_type"
+
+/* Map the chassis type from chassis_type to a sensible type used in hal
+ *
+ * See also 3.3.4.1 of the "System Management BIOS Reference Specification,
+ * Version 2.6.1" (Preliminary Standard) document, available from
+ * http://www.dmtf.org/standards/smbios.
+ *
+ * TODO: figure out WTF the mapping should be; "Lunch Box"? Give me a break :-)
+ *
+ * Copied from hal/hald/linux/osspec.c
+ */
+static const char *chassis_map[] = {
+ "Other", "unknown", /* 0x01 */
+ "Unknown", "unknown",
+ "Desktop", "desktop",
+ "Low Profile Desktop", "desktop",
+ "Pizza Box", "server",
+ "Mini Tower", "desktop",
+ "Tower", "desktop",
+ "Portable", "laptop",
+ "Laptop", "laptop",
+ "Notebook", "laptop",
+ "Hand Held", "handheld",
+ "Docking Station", "laptop",
+ "All In One", "unknown",
+ "Sub Notebook", "laptop",
+ "Space-saving", "desktop",
+ "Lunch Box", "unknown",
+ "Main Server Chassis", "server",
+ "Expansion Chassis", "unknown",
+ "Sub Chassis", "unknown",
+ "Bus Expansion Chassis", "unknown",
+ "Peripheral Chassis", "unknown",
+ "RAID Chassis", "unknown",
+ "Rack Mount Chassis", "unknown",
+ "Sealed-case PC", "unknown",
+ "Multi-system", "unknown",
+ "CompactPCI", "unknonw",
+ "AdvancedTCA", "unknown",
+ "Blade", "server",
+ "Blade Enclosure" "unknown", /* 0x1D */
+ NULL
+};
+
+static int formfactor_probe(struct btd_adapter *adapter)
+{
+ int chassis_type;
+ uint8_t minor = 0;
+ const char *formfactor;
+ char *contents;
+
+ if (g_file_get_contents(DMI_CHASSIS_FILE,
+ &contents, NULL, NULL) == FALSE) {
+ if (g_file_get_contents(DMI_CHASSIS_FILE_FALLBACK,
+ &contents, NULL, NULL) == FALSE) {
+ error("Could not get the contents of DMI chassis type");
+ return 0;
+ }
+ }
+
+ chassis_type = atoi(contents);
+ g_free (contents);
+
+ if (chassis_type > 0x1D || chassis_type <= 0) {
+ error ("Chassis type is not a known chassis type");
+ return 0;
+ }
+
+ formfactor = chassis_map[chassis_type * 2];
+ if (formfactor != NULL) {
+ if (g_str_equal(formfactor, "laptop") == TRUE)
+ minor |= (1 << 2) | (1 << 3);
+ else if (g_str_equal(formfactor, "desktop") == TRUE)
+ minor |= 1 << 2;
+ else if (g_str_equal(formfactor, "server") == TRUE)
+ minor |= 1 << 3;
+ else if (g_str_equal(formfactor, "handheld") == TRUE)
+ minor += 1 << 4;
+ }
+
+ /* Computer major class */
+ DBG("Setting 0x%06x for major/minor device class", (1 << 8) | minor);
+
+ btd_adapter_set_class(adapter, 0x01, minor);
+
+ return 0;
+}
+
+static void formfactor_remove(struct btd_adapter *adapter)
+{
+}
+
+static struct btd_adapter_driver formfactor_driver = {
+ .name = "formfactor",
+ .probe = formfactor_probe,
+ .remove = formfactor_remove,
+};
+
+static int formfactor_init(void)
+{
+ return btd_register_adapter_driver(&formfactor_driver);
+}
+
+static void formfactor_exit(void)
+{
+ btd_unregister_adapter_driver(&formfactor_driver);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(formfactor, VERSION,
+ BLUETOOTH_PLUGIN_PRIORITY_LOW, formfactor_init, formfactor_exit)
--
1.7.0.1
^ permalink raw reply related
* Re: Running "putkey" for keyboards and mice
From: Bastien Nocera @ 2010-09-06 12:19 UTC (permalink / raw)
To: Luiz Augusto von Dentz; +Cc: BlueZ development
In-Reply-To: <AANLkTim3SSZhhJOB0DfaOsxMEzsaNKe-dkEf59p-7UQv@mail.gmail.com>
On Mon, 2010-09-06 at 14:51 +0300, Luiz Augusto von Dentz wrote:
> Hi,
>
> On Mon, Sep 6, 2010 at 1:06 PM, Bastien Nocera <hadess@hadess.net> wrote:
> > Heya,
> >
> > Did a small test this week-end on my Macbook, and running "hciconfig"
> > with putkey works, adding the just paired keyboards' linkkey into the
> > Bluetooth adapter.
> >
> > This meant that even though I paired the device in Linux, it was still
> > available in MacOS X, without any more pairing.
> >
> > My question is whether we should, for all the adapters listed in the
> > hid2hci rules, copy the linkkeys for keyboards and (paired) mice[1] to
> > the adapter itself.
> >
> > How would you like to see this implemented? Would a device plugin be
> > good enough to track newly paired devices?
>
> Im not sure this is going to work the other way round, when you pair
> in some other SO and than save the key in the chip we would need to
> load this device as it is already paired.
Some Windows drivers support doing this, though not all of them. MacOS X
clearly supports this, as the recommended way to make sure a keyboard
will be available in Windows, when dual-booting with BootCamp, is to
boot into MacOS X, which will write the linkkeys to the device. So the
Windows Apple Bluetooth drivers know how to read the linkkeys, but not
write them.
> If that can be done than
> IMOH this should be integrated directly in core, perhaps adding
> .putkey/.delkey/.getkeys to hciops so we can properly use the keys
> stored on chip, but I guess we need to maintain our own storage too
> since chips normally have very limited space for storing keys.
Johan mentioned that we could implement this using a boolean property
for whether to store the linkkey on the device or not. When TRUE, should
we store the linkkey on both the device and on the local filesystem, or
just on the device?
hci_read_stored_link_key() is not currently used in the code at all, so
it's possible that a device will be paired, but not marked as such in
the interface.
Cheers
^ permalink raw reply
* Re: Add new plugin to set adapter class
From: Johan Hedberg @ 2010-09-06 13:13 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ development
In-Reply-To: <1283774756.7529.11.camel@localhost.localdomain>
Hi Bastien,
On Mon, Sep 06, 2010, Bastien Nocera wrote:
> OK. Patch that actually applies to master attached. Was conflicting with
> another non-upstreamed patch.
>
> From: Bastien Nocera <hadess@hadess.net>
> Date: Mon, 6 Sep 2010 11:44:31 +0100
> Subject: [PATCH] Add new plugin to set adapter class
>
> This time based on the kernel exported DMI, rather than calling
> out to HAL. The HAL plugin can still be preferred with
> --enable-hal=yes passed to configure.
> ---
> Makefile.am | 7 ++
> acinclude.m4 | 7 ++-
> plugins/formfactor.c | 151 ++++++++++++++++++++++++++++++++++++++++++++++++++
> 3 files changed, 164 insertions(+), 1 deletions(-)
> create mode 100644 plugins/formfactor.c
Thanks. The patch is now upstream.
Johan
^ permalink raw reply
* Re: [PATCH] Bluetooth: check L2CAP length in first ACL fragment
From: Andrei Emeltchenko @ 2010-09-06 13:25 UTC (permalink / raw)
To: Mat Martineau; +Cc: linux-bluetooth
In-Reply-To: <AANLkTi=bRRKPODUuoKepCRpH_SAJt3nSv=jWUBvDdJi0@mail.gmail.com>
Hi,
On Mon, Sep 6, 2010 at 2:32 PM, Andrei Emeltchenko
<andrei.emeltchenko.news@gmail.com> wrote:
> Hi,
>
> On Fri, Sep 3, 2010 at 7:26 PM, Mat Martineau <mathewm@codeaurora.org> wrote:
>>> Trace below is received when using stress tools sending big
>>> fragmented L2CAP packets.
>>> ...
>>> [ 1712.798492] swapper: page allocation failure. order:4, mode:0x4020
>>> [ 1712.804809] [<c0031870>] (unwind_backtrace+0x0/0xdc) from [<c00a1f70>]
>>> (__alloc_pages_nodemask+0x4)
>>> [ 1712.814666] [<c00a1f70>] (__alloc_pages_nodemask+0x47c/0x4d4) from
>>> [<c00a1fd8>] (__get_free_pages+)
>>> [ 1712.824645] [<c00a1fd8>] (__get_free_pages+0x10/0x3c) from [<c026eb5c>]
>>> (__alloc_skb+0x4c/0xfc)
>>> [ 1712.833465] [<c026eb5c>] (__alloc_skb+0x4c/0xfc) from [<bf28c738>]
>>> (l2cap_recv_acldata+0xf0/0x1f8 )
>>> [ 1712.843322] [<bf28c738>] (l2cap_recv_acldata+0xf0/0x1f8 [l2cap]) from
>>> [<bf0094ac>] (hci_rx_task+0x)
>>> ...
>>
>> What is logged right before this allocation failure? There should be a
>> "Start: total len %d, frag len %d" line. Actually, all log messages from
>> l2cap_recv_acldata leading up to the failure would be helpful.
>
> When I enable logs system behave differently because of huge traffic
> to syslog :-)
>
>>> After applying patch dmesg looks like:
>>> ...
>>> l2cap_recv_acldata: Frame exceeding recv MTU (len 64182, MTU 1013)
>>> l2cap_recv_acldata: Unexpected continuation frame (len 34)
>>> ...
>>>
>>> Signed-off-by: Andrei Emeltchenko <andrei.emeltchenko@nokia.com>
>>> ---
>>> net/bluetooth/l2cap.c | 11 +++++++++++
>>> 1 files changed, 11 insertions(+), 0 deletions(-)
>>>
>>> diff --git a/net/bluetooth/l2cap.c b/net/bluetooth/l2cap.c
>>> index 9fad312..21824d7 100644
>>> --- a/net/bluetooth/l2cap.c
>>> +++ b/net/bluetooth/l2cap.c
>>> @@ -4654,6 +4654,8 @@ static int l2cap_recv_acldata(struct hci_conn *hcon,
>>> struct sk_buff *skb, u16 fl
>>>
>>> if (flags & ACL_START) {
>>> struct l2cap_hdr *hdr;
>>> + struct sock *sk;
>>> + u16 cid;
>>> int len;
>>>
>>> if (conn->rx_len) {
>>> @@ -4672,6 +4674,7 @@ static int l2cap_recv_acldata(struct hci_conn *hcon,
>>> struct sk_buff *skb, u16 fl
>>> d
>>> hdr = (struct l2cap_hdr *) skb->data;
>>> len = __le16_to_cpu(hdr->len) + L2CAP_HDR_SIZE;
>>> + cid = __le16_to_cpu(hdr->cid);
>>
>> Some stress tools also send L2CAP data one byte at a time - the start
>> fragment may not have all four header bytes. l2cap_recv_acldata() currently
>> allows short start fragments with two or three bytes, which would not
>> contain a valid CID.
>
> Yes currently we allow 2 bytes in start fragment and check hdr->len.
> What about following change (require 4 bytes in start fragment)
>
> ---------------------------------------------
> - if (skb->len < 2) {
> + if (skb->len < L2CAP_HDR_SIZE) {
> BT_ERR("Frame is too short (len %d)", skb->len);
> l2cap_conn_unreliable(conn, ECOMM);
> goto drop;
> ---------------------------------------------
I have found in "BLUETOOTH SPECIFICATION Version 4.0 [Vol 3] page 36"
"Note: Start Fragments always begin with the Basic L2CAP header of a
PDU." So the statement above is correct.
Regards,
Andrei
>
>>> if (len == skb->len) {
>>> /* Complete frame received */
>>> @@ -4688,6 +4691,14 @@ static int l2cap_recv_acldata(struct hci_conn
>>> *hcon, struct sk_buff *skb, u16 fl
>>> goto drop;
>>> }
>>>
>>> + sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
>>
>> There are several issues here.
>>
>> l2cap_get_chan_by_scid() might return NULL, which will definitely happen
>> with signaling or connectionless data frames.
>>
>> l2cap_get_chan_by_scid() also calls bh_lock_sock() if a channel is found,
>> and I don't see that you added a matching call to bh_unlock_sock() if
>> l2cap_get_chan_by_scid() returns a non-NULL value.
>
> My mistake here. What about following check:
>
> ---------------------------------------
> @@ -3916,6 +3919,19 @@ static int l2cap_recv_acldata(struct hci_conn
> *hcon, struct sk_buff *skb,
> goto drop;
> }
>
> + sk = l2cap_get_chan_by_scid(&conn->chan_list, cid);
> + if (!sk)
> + goto drop;
> +
> + if (l2cap_pi(sk)->imtu < len) {
> + BT_ERR("Frame exceeding recv MTU (len %d, MTU %d)",
> + len, l2cap_pi(sk)->imtu);
> + conn->rx_len = 0; /* needed? */
> + bh_unlock_sock(sk);
> + goto drop;
> + } else
> + bh_unlock_sock(sk);
> +
> /* Allocate skb for the complete frame (with header) */
> conn->rx_skb = bt_skb_alloc(len, GFP_ATOMIC);
> if (!conn->rx_skb)
>
> --------------------------------------
>
>>
>>> + if (l2cap_pi(sk)->imtu < len) {
>>> + BT_ERR("Frame exceeding recv MTU (len %d, MTU
>>> %d)",
>>> + len, l2cap_pi(sk)->imtu);
>>> + conn->rx_len = 0; /* needed? */
>>> + goto drop;
>>> + }
>>> +
>>> /* Allocate skb for the complete frame (with header) */
>>> conn->rx_skb = bt_skb_alloc(len, GFP_ATOMIC);
>>> if (!conn->rx_skb)
>>
>> In general, this approach adds an extra channel lookup and an extra
>> lock/unlock on every ACL start frame.
>
> Yes this adds extra lock/unlock but only if the frame is not complete. There
> shouldn't be many fragmented L2CAP packets.
>
>> Even if the MTU is known to be
>> exceeded on with the start frame, no more than 64k is ever allocated (which
>> shouldn't cause problems in itself).
>
> I think that for the mobile device this can be a problem since this
> shall be contiguous
> memory.
>
>> The root of the problem may be heap
>> corruption due to a buffer overrun, which seems possible due to the crash
>> deep in the memory allocator.
>
> Regards,
> Andrei
>
^ permalink raw reply
* Re: [PATCH 3/4] Fix calling watch callbacks after it has been removed
From: Luiz Augusto von Dentz @ 2010-09-06 13:39 UTC (permalink / raw)
To: linux-bluetooth
In-Reply-To: <1283768782-4283-3-git-send-email-luiz.dentz@gmail.com>
This one is broken, I will resubmit.
On Mon, Sep 6, 2010 at 1:26 PM, Luiz Augusto von Dentz
<luiz.dentz@gmail.com> wrote:
> From: Luiz Augusto Von Dentz <luiz.dentz-von@nokia.com>
>
> Pending call should be removed if the watch is removed since the
> application no longer expect that to be reached and may already freed the
> data associated with it.
> ---
> gdbus/watch.c | 75 ++++++++++++++++++++++++++++++++++++--------------------
> 1 files changed, 48 insertions(+), 27 deletions(-)
>
> diff --git a/gdbus/watch.c b/gdbus/watch.c
> index 8ad4815..249d927 100644
> --- a/gdbus/watch.c
> +++ b/gdbus/watch.c
> @@ -43,11 +43,21 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
> static guint listener_id = 0;
> static GSList *listeners = NULL;
>
> +struct service_data {
> + DBusConnection *conn;
> + DBusPendingCall *call;
> + char *name;
> + const char *owner;
> + guint id;
> + struct filter_callback *callback;
> +};
> +
> struct filter_callback {
> GDBusWatchFunction conn_func;
> GDBusWatchFunction disc_func;
> GDBusSignalFunction signal_func;
> GDBusDestroyFunction destroy_func;
> + struct service_data *data;
> void *user_data;
> guint id;
> };
> @@ -302,7 +312,7 @@ static struct filter_callback *filter_data_add_callback(
> {
> struct filter_callback *cb = NULL;
>
> - cb = g_new(struct filter_callback, 1);
> + cb = g_new0(struct filter_callback, 1);
>
> cb->conn_func = connect;
> cb->disc_func = disconnect;
> @@ -319,6 +329,24 @@ static struct filter_callback *filter_data_add_callback(
> return cb;
> }
>
> +static void service_data_free(struct service_data *data)
> +{
> + struct filter_callback *callback = data->callback;
> +
> + dbus_connection_unref(data->conn);
> +
> + if (data->call)
> + dbus_pending_call_unref(data->call);
> +
> + if (data->id)
> + g_source_remove(data->id);
> +
> + g_free(data->name);
> + g_free(data);
> +
> + callback->data = NULL;
> +}
> +
> static gboolean filter_data_remove_callback(struct filter_data *data,
> struct filter_callback *cb)
> {
> @@ -327,6 +355,13 @@ static gboolean filter_data_remove_callback(struct filter_data *data,
> data->callbacks = g_slist_remove(data->callbacks, cb);
> data->processed = g_slist_remove(data->processed, cb);
>
> + /* Cancel pending operations */
> + if (cb->data) {
> + if (cb->data->call)
> + dbus_pending_call_cancel(cb->data->call);
> + service_data_free(cb->data);
> + }
> +
> if (cb->destroy_func)
> cb->destroy_func(cb->user_data);
>
> @@ -515,28 +550,14 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
> return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
> }
>
> -struct service_data {
> - DBusConnection *conn;
> - char *name;
> - const char *owner;
> - GDBusWatchFunction conn_func;
> - void *user_data;
> -};
> -
> -static void service_data_free(struct service_data *data)
> -{
> - dbus_connection_unref(data->conn);
> - g_free(data->name);
> - g_free(data);
> -}
> -
> static gboolean update_service(void *user_data)
> {
> struct service_data *data = user_data;
> + struct filter_callback *cb = data->callback;
>
> update_name_cache(data->name, data->owner);
> - if (data->conn_func)
> - data->conn_func(data->conn, data->user_data);
> + if (cb->conn_func)
> + cb->conn_func(data->conn, cb->user_data);
>
> service_data_free(data);
>
> @@ -575,11 +596,11 @@ done:
> dbus_message_unref(reply);
> }
>
> -static void check_service(DBusConnection *connection, const char *name,
> - GDBusWatchFunction connect, void *user_data)
> +static void check_service(DBusConnection *connection,
> + const char *name,
> + struct filter_callback *callback)
> {
> DBusMessage *message;
> - DBusPendingCall *call;
> struct service_data *data;
>
> data = g_try_malloc0(sizeof(*data));
> @@ -590,12 +611,12 @@ static void check_service(DBusConnection *connection, const char *name,
>
> data->conn = dbus_connection_ref(connection);
> data->name = g_strdup(name);
> - data->conn_func = connect;
> - data->user_data = user_data;
> + data->callback = callback;
> + callback->data = data;
>
> data->owner = check_name_cache(name);
> if (data->owner != NULL) {
> - g_idle_add(update_service, data);
> + data->id = g_idle_add(update_service, data);
> return;
> }
>
> @@ -611,13 +632,13 @@ static void check_service(DBusConnection *connection, const char *name,
> DBUS_TYPE_INVALID);
>
> if (dbus_connection_send_with_reply(connection, message,
> - &call, -1) == FALSE) {
> + &data->call, -1) == FALSE) {
> error("Failed to execute method call");
> g_free(data);
> goto done;
> }
>
> - if (call == NULL) {
> + if (data->call == NULL) {
> error("D-Bus connection not available");
> g_free(data);
> goto done;
> @@ -654,7 +675,7 @@ guint g_dbus_add_service_watch(DBusConnection *connection, const char *name,
> return 0;
>
> if (connect)
> - check_service(connection, name, connect, user_data);
> + check_service(connection, name, cb);
>
> return cb->id;
> }
> --
> 1.7.0.4
>
> --
> To unsubscribe from this list: send the line "unsubscribe linux-bluetooth" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at http://vger.kernel.org/majordomo-info.html
>
--
Luiz Augusto von Dentz
Computer Engineer
^ permalink raw reply
* [PATCH 3/4] Fix calling watch callbacks after it has been removed
From: Luiz Augusto von Dentz @ 2010-09-06 13:39 UTC (permalink / raw)
To: linux-bluetooth
From: Luiz Augusto von Dentz <luiz.dentz-von@nokia.com>
Pending call should be removed if the watch is removed since the
application no longer expect that to be reached and may already freed the
data associated with it.
---
gdbus/watch.c | 79 +++++++++++++++++++++++++++++++++++---------------------
1 files changed, 49 insertions(+), 30 deletions(-)
diff --git a/gdbus/watch.c b/gdbus/watch.c
index 8ad4815..c0dcc93 100644
--- a/gdbus/watch.c
+++ b/gdbus/watch.c
@@ -43,11 +43,21 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
static guint listener_id = 0;
static GSList *listeners = NULL;
+struct service_data {
+ DBusConnection *conn;
+ DBusPendingCall *call;
+ char *name;
+ const char *owner;
+ guint id;
+ struct filter_callback *callback;
+};
+
struct filter_callback {
GDBusWatchFunction conn_func;
GDBusWatchFunction disc_func;
GDBusSignalFunction signal_func;
GDBusDestroyFunction destroy_func;
+ struct service_data *data;
void *user_data;
guint id;
};
@@ -302,7 +312,7 @@ static struct filter_callback *filter_data_add_callback(
{
struct filter_callback *cb = NULL;
- cb = g_new(struct filter_callback, 1);
+ cb = g_new0(struct filter_callback, 1);
cb->conn_func = connect;
cb->disc_func = disconnect;
@@ -319,6 +329,24 @@ static struct filter_callback *filter_data_add_callback(
return cb;
}
+static void service_data_free(struct service_data *data)
+{
+ struct filter_callback *callback = data->callback;
+
+ dbus_connection_unref(data->conn);
+
+ if (data->call)
+ dbus_pending_call_unref(data->call);
+
+ if (data->id)
+ g_source_remove(data->id);
+
+ g_free(data->name);
+ g_free(data);
+
+ callback->data = NULL;
+}
+
static gboolean filter_data_remove_callback(struct filter_data *data,
struct filter_callback *cb)
{
@@ -327,6 +355,13 @@ static gboolean filter_data_remove_callback(struct filter_data *data,
data->callbacks = g_slist_remove(data->callbacks, cb);
data->processed = g_slist_remove(data->processed, cb);
+ /* Cancel pending operations */
+ if (cb->data) {
+ if (cb->data->call)
+ dbus_pending_call_cancel(cb->data->call);
+ service_data_free(cb->data);
+ }
+
if (cb->destroy_func)
cb->destroy_func(cb->user_data);
@@ -515,28 +550,14 @@ static DBusHandlerResult message_filter(DBusConnection *connection,
return DBUS_HANDLER_RESULT_NOT_YET_HANDLED;
}
-struct service_data {
- DBusConnection *conn;
- char *name;
- const char *owner;
- GDBusWatchFunction conn_func;
- void *user_data;
-};
-
-static void service_data_free(struct service_data *data)
-{
- dbus_connection_unref(data->conn);
- g_free(data->name);
- g_free(data);
-}
-
static gboolean update_service(void *user_data)
{
struct service_data *data = user_data;
+ struct filter_callback *cb = data->callback;
update_name_cache(data->name, data->owner);
- if (data->conn_func)
- data->conn_func(data->conn, data->user_data);
+ if (cb->conn_func)
+ cb->conn_func(data->conn, cb->user_data);
service_data_free(data);
@@ -575,11 +596,11 @@ done:
dbus_message_unref(reply);
}
-static void check_service(DBusConnection *connection, const char *name,
- GDBusWatchFunction connect, void *user_data)
+static void check_service(DBusConnection *connection,
+ const char *name,
+ struct filter_callback *callback)
{
DBusMessage *message;
- DBusPendingCall *call;
struct service_data *data;
data = g_try_malloc0(sizeof(*data));
@@ -590,12 +611,12 @@ static void check_service(DBusConnection *connection, const char *name,
data->conn = dbus_connection_ref(connection);
data->name = g_strdup(name);
- data->conn_func = connect;
- data->user_data = user_data;
+ data->callback = callback;
+ callback->data = data;
data->owner = check_name_cache(name);
if (data->owner != NULL) {
- g_idle_add(update_service, data);
+ data->id = g_idle_add(update_service, data);
return;
}
@@ -611,21 +632,19 @@ static void check_service(DBusConnection *connection, const char *name,
DBUS_TYPE_INVALID);
if (dbus_connection_send_with_reply(connection, message,
- &call, -1) == FALSE) {
+ &data->call, -1) == FALSE) {
error("Failed to execute method call");
g_free(data);
goto done;
}
- if (call == NULL) {
+ if (data->call == NULL) {
error("D-Bus connection not available");
g_free(data);
goto done;
}
- dbus_pending_call_set_notify(call, service_reply, data, g_free);
-
- dbus_pending_call_unref(call);
+ dbus_pending_call_set_notify(data->call, service_reply, data, NULL);
done:
dbus_message_unref(message);
@@ -654,7 +673,7 @@ guint g_dbus_add_service_watch(DBusConnection *connection, const char *name,
return 0;
if (connect)
- check_service(connection, name, connect, user_data);
+ check_service(connection, name, cb);
return cb->id;
}
--
1.7.0.4
^ permalink raw reply related
* [PATCH] Make the path to oui.txt a compile-time option
From: Bastien Nocera @ 2010-09-06 14:47 UTC (permalink / raw)
To: BlueZ development
[-- Attachment #1: Type: text/plain, Size: 199 bytes --]
Heya,
We've been carrying a similar (but hard-coded) patch in Fedora for a
long while, and it would be nice if we could do this properly upstream
instead.
Patch attached with explanations.
Cheers
[-- Attachment #2: 0001-Make-the-path-to-oui.txt-a-compile-time-option.patch --]
[-- Type: text/x-patch, Size: 1927 bytes --]
>From b96d9394aa850be86ca368aeaeec3bc24c00bce1 Mon Sep 17 00:00:00 2001
From: Bastien Nocera <hadess@hadess.net>
Date: Mon, 6 Sep 2010 15:43:48 +0100
Subject: [PATCH] Make the path to oui.txt a compile-time option
This avoids bluetoothd trying to load oui.txt from multiple locations
and cause SELinux AVC denials by accessing files it's not supposed
to touch.
---
acinclude.m4 | 8 ++++++++
configure.ac | 1 +
src/oui.c | 14 +++-----------
3 files changed, 12 insertions(+), 11 deletions(-)
diff --git a/acinclude.m4 b/acinclude.m4
index b34f08d..a4e5e2f 100644
--- a/acinclude.m4
+++ b/acinclude.m4
@@ -152,6 +152,14 @@ AC_DEFUN([AC_PATH_SNDFILE], [
AC_SUBST(SNDFILE_LIBS)
])
+AC_DEFUN([AC_PATH_OUI], [
+ AC_ARG_WITH(ouifile,
+ AS_HELP_STRING([--with-ouifile=PATH],[Path to the oui.txt file @<:@auto@:>@]),
+ [ac_with_ouifile=$withval],
+ [ac_with_ouifile="/var/lib/misc/oui.txt"])
+ AC_DEFINE_UNQUOTED(OUIFILE, ["$ac_with_ouifile"], [Define the OUI file path])
+])
+
AC_DEFUN([AC_ARG_BLUEZ], [
debug_enable=no
optimization_enable=yes
diff --git a/configure.ac b/configure.ac
index e8abfe9..74acf31 100644
--- a/configure.ac
+++ b/configure.ac
@@ -41,6 +41,7 @@ AC_PATH_ALSA
AC_PATH_GSTREAMER
AC_PATH_USB
AC_PATH_SNDFILE
+AC_PATH_OUI
AC_ARG_BLUEZ
diff --git a/src/oui.c b/src/oui.c
index 1096d20..80bb3d3 100644
--- a/src/oui.c
+++ b/src/oui.c
@@ -38,23 +38,15 @@
/* http://standards.ieee.org/regauth/oui/oui.txt */
-#define OUIFILE "/var/lib/misc/oui.txt"
-
char *ouitocomp(const char *oui)
{
struct stat st;
char *str, *map, *off, *end;
int fd;
- fd = open("oui.txt", O_RDONLY);
- if (fd < 0) {
- fd = open(OUIFILE, O_RDONLY);
- if (fd < 0) {
- fd = open("/usr/share/misc/oui.txt", O_RDONLY);
- if (fd < 0)
- return NULL;
- }
- }
+ fd = open(OUIFILE, O_RDONLY);
+ if (fd < 0)
+ return NULL;
if (fstat(fd, &st) < 0) {
close(fd);
--
1.7.0.1
^ permalink raw reply related
* Re: [PATCH] Make the path to oui.txt a compile-time option
From: Pacho Ramos @ 2010-09-06 15:12 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ development
In-Reply-To: <1283784432.7529.24.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 381 bytes --]
El lun, 06-09-2010 a las 15:47 +0100, Bastien Nocera escribió:
> Heya,
>
> We've been carrying a similar (but hard-coded) patch in Fedora for a
> long while, and it would be nice if we could do this properly upstream
> instead.
>
> Patch attached with explanations.
>
> Cheers
What is the proper package to ship oui.txt file? Thanks a lot for the
information :-)
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
* Re: [PATCH] Make the path to oui.txt a compile-time option
From: Bastien Nocera @ 2010-09-06 15:15 UTC (permalink / raw)
To: pacho; +Cc: BlueZ development
In-Reply-To: <1283785976.28657.1.camel@localhost.localdomain>
On Mon, 2010-09-06 at 17:12 +0200, Pacho Ramos wrote:
> El lun, 06-09-2010 a las 15:47 +0100, Bastien Nocera escribió:
> > Heya,
> >
> > We've been carrying a similar (but hard-coded) patch in Fedora for a
> > long while, and it would be nice if we could do this properly upstream
> > instead.
> >
> > Patch attached with explanations.
> >
> > Cheers
>
> What is the proper package to ship oui.txt file? Thanks a lot for the
> information :-)
No idea. In Fedora we ship it in hwdata. No idea where it lives for
Debian-based distributions, or others.
^ permalink raw reply
* Re: [PATCH] Make the path to oui.txt a compile-time option
From: Pacho Ramos @ 2010-09-06 15:16 UTC (permalink / raw)
To: Bastien Nocera; +Cc: BlueZ development
In-Reply-To: <1283786114.7529.25.camel@localhost.localdomain>
[-- Attachment #1: Type: text/plain, Size: 857 bytes --]
El lun, 06-09-2010 a las 16:15 +0100, Bastien Nocera escribió:
> On Mon, 2010-09-06 at 17:12 +0200, Pacho Ramos wrote:
> > El lun, 06-09-2010 a las 15:47 +0100, Bastien Nocera escribió:
> > > Heya,
> > >
> > > We've been carrying a similar (but hard-coded) patch in Fedora for a
> > > long while, and it would be nice if we could do this properly upstream
> > > instead.
> > >
> > > Patch attached with explanations.
> > >
> > > Cheers
> >
> > What is the proper package to ship oui.txt file? Thanks a lot for the
> > information :-)
>
> No idea. In Fedora we ship it in hwdata. No idea where it lives for
> Debian-based distributions, or others.
>
OK, thanks, I asked it because under Gentoo no package is currently
providing it and a user suggested to supply it with bluez... but I
haven't made any decision about this yet
[-- Attachment #2: This is a digitally signed message part --]
[-- Type: application/pgp-signature, Size: 198 bytes --]
^ permalink raw reply
page: next (older) | prev (newer) | latest
- recent:[subjects (threaded)|topics (new)|topics (active)]
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox