Re: [Bluez-devel] [PATCH] client.c cleanup

Bastien Nocera
2008-01-31T14:57:52Z

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

On Wed, 2008-01-30 at 17:31 +0000, Bastien Nocera wrote:
> On Tue, 2008-01-29 at 18:01 +0100, Marcel Holtmann wrote:
> > Hi Bastien,
> >
> > > Some time ago I had problems with client.c's architecture. It would
> > > allow me to easily report completed discoveries without causing crashes.
> > >
> > > The attached patch moves all the static variables into the main
> > > BluetoothClient object, and instead of return a new object when using
> > > _new() we return an existing instance of the client.
> > >
> > > This makes no changes to the external API for the BluetoothClient, and
> > > should work just as well as older versions did.
> >
> > patch has been applied. Don't really see the difference, but I am okay
> > with doing it this way.
> >
> > > A couple of bugs I found in client.c:
> > > - launch test-client with a dongle inserted, remove and reinsert the
> > > dongle, the bdaddr of the device is not set anymore
> > > - launch test-client without a dongle inserted, insert it, the bdaddr is
> > > not set and there's no children to the adapter (although there are if
> > > inserted when started)
>
> Both of those are a race:
> When the AdapterAdded signal is launched in hcid, the address isn't
> available yet. It is available when "ModeChanged" has been launched
> though.
>
> hcid_dbus_register_device() setups the structures, and launched
> AdapterAdded, but the address is only set when hcid_dbus_start_device()
> is called (and the ModeChanged signal is launched).
>
> How do you think we should be handling that? I would think we should
> make GetAddress fail if the data isn't available yet, or hack around it
> in client.c and only setup the adapters when the mode changes.

The easy fix for hcid attached (the default bdaddr should be
00:00:00:00:00:00:00, not an empty string).

And also a patch for client.c to get the device address again when the
device changes mode, so we can change it in the tree from the default
all-zeros address to the real one.

I'll fix the other bugs now.

Cheers

[-- Attachment #2: bluez-utils-hcid-set-default-address.patch --]
[-- Type: text/x-patch, Size: 542 bytes --]

Index: dbus-hci.c
===================================================================
RCS file: /cvsroot/bluez/utils/hcid/dbus-hci.c,v
retrieving revision 1.35
diff -u -p -r1.35 dbus-hci.c
--- dbus-hci.c 28 Jan 2008 10:38:40 -0000 1.35
+++ dbus-hci.c 31 Jan 2008 14:41:40 -0000
@@ -477,6 +477,7 @@ int hcid_dbus_register_device(uint16_t i

adapter->dev_id = id;
adapter->pdiscov_resolve_names = 1;
+ strcpy(adapter->address, "00:00:00:00:00:00:00");

if (!dbus_connection_create_object_path(connection, path, adapter,
NULL)) {

[-- Attachment #3: bluez-gnome-update-bdaddr.patch --]
[-- Type: text/x-patch, Size: 1442 bytes --]

Index: client.c
===================================================================
RCS file: /cvsroot/bluez/gnome/common/client.c,v
retrieving revision 1.37
diff -u -p -r1.37 client.c
--- client.c 29 Jan 2008 17:00:58 -0000 1.37
+++ client.c 31 Jan 2008 14:56:56 -0000
@@ -60,6 +60,8 @@ static int client_table_signals[LAST_SIG

G_DEFINE_TYPE(BluetoothClient, bluetooth_client, G_TYPE_OBJECT)

+static void update_adapter(DBusGProxy *object, GtkTreeIter *iter, BluetoothClient *client);
+
static gboolean find_iter_for_object(DBusGProxy *object,
BluetoothClient *client,
GtkTreeIter *iter)
@@ -87,6 +89,29 @@ static gboolean find_iter_for_object(DBu
static void mode_changed(DBusGProxy *object,
const char *mode, gpointer user_data)
{
+ BluetoothClient *client = (BluetoothClient *) user_data;
+ BluetoothClientPrivate *priv = BLUETOOTH_CLIENT_GET_PRIVATE(client);
+ GtkTreeIter iter;
+ gboolean cont;
+
+ cont = gtk_tree_model_get_iter_first(GTK_TREE_MODEL(priv->store), &iter);
+
+ while (cont == TRUE) {
+ DBusGProxy *o;
+
+ gtk_tree_model_get(GTK_TREE_MODEL(priv->store), &iter,
+ COLUMN_OBJECT, &o, -1);
+
+ if (object == o) {
+ gtk_tree_store_set(priv->store, &iter,
+ COLUMN_ACTIVE, TRUE, -1);
+ update_adapter(object, &iter, client);
+
+ return;
+ }
+
+ cont = gtk_tree_model_iter_next(GTK_TREE_MODEL(priv->store), &iter);
+ }
}

static void timeout_changed(DBusGProxy *object,

[-- Attachment #4: Type: text/plain, Size: 228 bytes --]

-------------------------------------------------------------------------
This SF.net email is sponsored by: Microsoft
Defy all challenges. Microsoft(R) Visual Studio 2008.
http://clk.atdmt.com/MRT/go/vse0120000070mrt/direct/01/

[-- Attachment #5: Type: text/plain, Size: 164 bytes --]