linux-bluetooth.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
* [PATCH 1/3] Simplify "SetName" D-Bus call
@ 2011-06-22 11:29 Bastien Nocera
  2011-06-22 11:29 ` [PATCH 2/3] Add ability to block name changes Bastien Nocera
  2011-06-22 11:29 ` [PATCH 3/3] Add adaptername plugin Bastien Nocera
  0 siblings, 2 replies; 9+ messages in thread
From: Bastien Nocera @ 2011-06-22 11:29 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

By merging most of the functionality into adapter_update_local_name()
This also allows us to drop the adapter->name_stored struct member
as in any calls to adapter_update_local_name(), we would want to
write out the adapter name.
---
 src/adapter.c |   61 ++++++++++++++++++++++++--------------------------------
 src/adapter.h |    2 +-
 2 files changed, 27 insertions(+), 36 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index 2650b56..f8c0370 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -146,8 +146,6 @@ struct btd_adapter {
 
 	GSList *powered_callbacks;
 
-	gboolean name_stored;
-
 	GSList *loaded_drivers;
 };
 
@@ -914,10 +912,17 @@ void btd_adapter_class_changed(struct btd_adapter *adapter, uint32_t new_class)
 				DBUS_TYPE_UINT32, &new_class);
 }
 
-void adapter_update_local_name(struct btd_adapter *adapter, const char *name)
+int adapter_update_local_name(struct btd_adapter *adapter, const char *name)
 {
+	char *name_ptr;
+
 	if (strncmp(name, adapter->name, MAX_NAME_LENGTH) == 0)
-		return;
+		return 0;
+
+	if (!g_utf8_validate(name, -1, NULL)) {
+		error("Name change failed: supplied name isn't valid UTF-8");
+		return -EINVAL;
+	}
 
 	strncpy(adapter->name, name, MAX_NAME_LENGTH);
 
@@ -925,49 +930,36 @@ void adapter_update_local_name(struct btd_adapter *adapter, const char *name)
 		attrib_gap_set(GATT_CHARAC_DEVICE_NAME,
 			(const uint8_t *) adapter->name, strlen(adapter->name));
 
-	if (!adapter->name_stored) {
-		char *name_ptr = adapter->name;
+	name_ptr = adapter->name;
 
-		write_local_name(&adapter->bdaddr, adapter->name);
+	write_local_name(&adapter->bdaddr, adapter->name);
 
-		if (connection)
-			emit_property_changed(connection, adapter->path,
-						ADAPTER_INTERFACE, "Name",
-						DBUS_TYPE_STRING, &name_ptr);
+	if (connection)
+		emit_property_changed(connection, adapter->path,
+				      ADAPTER_INTERFACE, "Name",
+				      DBUS_TYPE_STRING, &name_ptr);
+
+	if (adapter->up) {
+		int err = adapter_ops->set_name(adapter->dev_id, name);
+		if (err < 0)
+			return err;
 	}
 
-	adapter->name_stored = FALSE;
+	return 0;
 }
 
 static DBusMessage *set_name(DBusConnection *conn, DBusMessage *msg,
 					const char *name, void *data)
 {
 	struct btd_adapter *adapter = data;
-	char *name_ptr = adapter->name;
+	int ret;
 
-	if (!g_utf8_validate(name, -1, NULL)) {
-		error("Name change failed: supplied name isn't valid UTF-8");
+	ret = adapter_update_local_name(adapter, name);
+	if (ret == -EINVAL)
 		return btd_error_invalid_args(msg);
-	}
-
-	if (strncmp(name, adapter->name, MAX_NAME_LENGTH) == 0)
-		goto done;
-
-	strncpy(adapter->name, name, MAX_NAME_LENGTH);
-	write_local_name(&adapter->bdaddr, name);
-	emit_property_changed(connection, adapter->path,
-					ADAPTER_INTERFACE, "Name",
-					DBUS_TYPE_STRING, &name_ptr);
+	else if (ret < 0)
+		return btd_error_failed(msg, strerror(-ret));
 
-	if (adapter->up) {
-		int err = adapter_ops->set_name(adapter->dev_id, name);
-		if (err < 0)
-			return btd_error_failed(msg, strerror(-err));
-
-		adapter->name_stored = TRUE;
-	}
-
-done:
 	return dbus_message_new_method_return(msg);
 }
 
@@ -2496,7 +2488,6 @@ int btd_adapter_stop(struct btd_adapter *adapter)
 	adapter->mode = MODE_OFF;
 	adapter->state = STATE_IDLE;
 	adapter->off_requested = FALSE;
-	adapter->name_stored = FALSE;
 
 	call_adapter_powered_callbacks(adapter, FALSE);
 
diff --git a/src/adapter.h b/src/adapter.h
index 610331f..9516a99 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -115,7 +115,7 @@ int adapter_remove_found_device(struct btd_adapter *adapter, bdaddr_t *bdaddr);
 void adapter_emit_device_found(struct btd_adapter *adapter,
 						struct remote_dev_info *dev);
 void adapter_mode_changed(struct btd_adapter *adapter, uint8_t scan_mode);
-void adapter_update_local_name(struct btd_adapter *adapter, const char *name);
+int adapter_update_local_name(struct btd_adapter *adapter, const char *name);
 void adapter_service_insert(struct btd_adapter *adapter, void *rec);
 void adapter_service_remove(struct btd_adapter *adapter, void *rec);
 void btd_adapter_class_changed(struct btd_adapter *adapter,
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 2/3] Add ability to block name changes
  2011-06-22 11:29 [PATCH 1/3] Simplify "SetName" D-Bus call Bastien Nocera
@ 2011-06-22 11:29 ` Bastien Nocera
  2011-06-22 11:29 ` [PATCH 3/3] Add adaptername plugin Bastien Nocera
  1 sibling, 0 replies; 9+ messages in thread
From: Bastien Nocera @ 2011-06-22 11:29 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

---
 src/adapter.c |   12 ++++++++++++
 src/adapter.h |    2 ++
 2 files changed, 14 insertions(+), 0 deletions(-)

diff --git a/src/adapter.c b/src/adapter.c
index f8c0370..5420acd 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -111,6 +111,7 @@ struct btd_adapter {
 	bdaddr_t bdaddr;		/* adapter Bluetooth Address */
 	uint32_t dev_class;		/* Class of Device */
 	char name[MAX_NAME_LENGTH + 1]; /* adapter name */
+	gboolean allow_name_changes;	/* whether the adapter name can be changed */
 	guint discov_timeout_id;	/* discoverable timeout id */
 	guint stop_discov_id;		/* stop inquiry/scanning id */
 	uint32_t discov_timeout;	/* discoverable time(sec) */
@@ -916,6 +917,9 @@ int adapter_update_local_name(struct btd_adapter *adapter, const char *name)
 {
 	char *name_ptr;
 
+	if (adapter->allow_name_changes == FALSE)
+		return -EPERM;
+
 	if (strncmp(name, adapter->name, MAX_NAME_LENGTH) == 0)
 		return 0;
 
@@ -2563,6 +2567,8 @@ gboolean adapter_init(struct btd_adapter *adapter)
 	 * start off as powered */
 	adapter->up = TRUE;
 
+	adapter->allow_name_changes = TRUE;
+
 	adapter_ops->read_bdaddr(adapter->dev_id, &adapter->bdaddr);
 
 	if (bacmp(&adapter->bdaddr, BDADDR_ANY) == 0) {
@@ -2663,6 +2669,12 @@ void adapter_get_address(struct btd_adapter *adapter, bdaddr_t *bdaddr)
 	bacpy(bdaddr, &adapter->bdaddr);
 }
 
+void adapter_set_allow_name_changes(struct btd_adapter *adapter,
+						gboolean allow_name_changes)
+{
+	adapter->allow_name_changes = allow_name_changes;
+}
+
 static inline void suspend_discovery(struct btd_adapter *adapter)
 {
 	if (adapter->state != STATE_SUSPENDED)
diff --git a/src/adapter.h b/src/adapter.h
index 9516a99..38ea3ca 100644
--- a/src/adapter.h
+++ b/src/adapter.h
@@ -100,6 +100,8 @@ int adapter_resolve_names(struct btd_adapter *adapter);
 struct btd_adapter *adapter_create(DBusConnection *conn, int id);
 gboolean adapter_init(struct btd_adapter *adapter);
 void adapter_remove(struct btd_adapter *adapter);
+void adapter_set_allow_name_changes(struct btd_adapter *adapter,
+						gboolean allow_name_changes);
 uint16_t adapter_get_dev_id(struct btd_adapter *adapter);
 const gchar *adapter_get_path(struct btd_adapter *adapter);
 void adapter_get_address(struct btd_adapter *adapter, bdaddr_t *bdaddr);
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH 3/3] Add adaptername plugin
  2011-06-22 11:29 [PATCH 1/3] Simplify "SetName" D-Bus call Bastien Nocera
  2011-06-22 11:29 ` [PATCH 2/3] Add ability to block name changes Bastien Nocera
@ 2011-06-22 11:29 ` Bastien Nocera
  2011-06-23 13:42   ` [PATCH] " Bastien Nocera
  1 sibling, 1 reply; 9+ messages in thread
From: Bastien Nocera @ 2011-06-22 11:29 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Replacing the name setting code in src/adapter.c.

Moving the adapter naming allows us to use the /etc/machine-info [1]
pretty hostname, as implemented by hostnamed [2] in systemd.

If /etc/machine-info is not present, the adapter name stored
on disk in /var/lib/bluetooth will be used. If no adapter name
has been set yet, the default from the main.conf will be used.

We don't currently number the name of hci0 if a pretty name is
available, but we should instead number it if it happens not
to be the default adapter. As we cannot be told when the default
adapter changes, we'll behave this way for now.

Note that when an adapter name is set automatically from
the pretty hostname, changing it through the D-Bus interface
will fail.

[1]: http://0pointer.de/public/systemd-man/machine-info.html
[2]: http://www.freedesktop.org/wiki/Software/systemd/hostnamed
---
 Makefile.am           |    3 +
 configure.ac          |    4 +
 plugins/adaptername.c |  303 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/adapter.c         |   66 -----------
 4 files changed, 310 insertions(+), 66 deletions(-)
 create mode 100644 plugins/adaptername.c

diff --git a/Makefile.am b/Makefile.am
index c2a6976..ba9a89d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -218,6 +218,9 @@ EXTRA_DIST += plugins/hal.c plugins/formfactor.c
 builtin_modules += storage
 builtin_sources += plugins/storage.c
 
+builtin_modules += adaptername
+builtin_sources += plugins/adaptername.c
+
 if MAEMO6PLUGIN
 builtin_modules += maemo6
 builtin_sources += plugins/maemo6.c
diff --git a/configure.ac b/configure.ac
index 07b9e55..084ece5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -35,6 +35,10 @@ AC_FUNC_PPOLL
 AC_CHECK_LIB(dl, dlopen, dummy=yes,
 			AC_MSG_ERROR(dynamic linking loader is required))
 
+AC_CHECK_HEADER([sys/inotify.h],
+		[AC_DEFINE([HAVE_SYS_INOTIFY_H], 1,
+			   [Define to 1 if you have <sys/inotify.h>.])],
+			   [AC_MSG_ERROR(inotify headers are required and missing)])
 AC_PATH_DBUS
 AC_PATH_GLIB
 AC_PATH_ALSA
diff --git a/plugins/adaptername.c b/plugins/adaptername.c
new file mode 100644
index 0000000..d7eef57
--- /dev/null
+++ b/plugins/adaptername.c
@@ -0,0 +1,303 @@
+/*
+ *
+ *  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 <unistd.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <bluetooth/bluetooth.h>
+
+#include "plugin.h"
+#include "hcid.h" /* For main_opts */
+#include "adapter.h"
+#include "manager.h"
+#include "device.h" /* Needed for storage.h */
+#include "storage.h"
+#include "log.h"
+
+#include <sys/inotify.h>
+#define EVENT_SIZE  (sizeof (struct inotify_event))
+#define EVENT_BUF_LEN (1024 * (EVENT_SIZE + 16))
+
+#define MACHINE_INFO_DIR "/etc/"
+#define MACHINE_INFO_FILE "machine-info"
+
+static GIOChannel *inotify = NULL;
+static int watch_fd = -1;
+
+/* This file is part of systemd's hostnamed functionality:
+ * http://0pointer.de/public/systemd-man/machine-info.html
+ * http://www.freedesktop.org/wiki/Software/systemd/hostnamed
+ */
+static char *read_pretty_host_name(void)
+{
+	char *contents, *ret;
+	char **lines;
+	guint i;
+
+	if (g_file_get_contents(MACHINE_INFO_DIR MACHINE_INFO_FILE,
+			&contents, NULL, NULL) == FALSE) {
+		return NULL;
+	}
+	lines = g_strsplit_set(contents, "\r\n", 0);
+	g_free(contents);
+
+	if (lines == NULL)
+		return NULL;
+
+	ret = NULL;
+	for (i = 0; lines[i] != NULL; i++) {
+		if (g_str_has_prefix(lines[i], "PRETTY_HOSTNAME=")) {
+			ret = g_strdup(lines[i] + strlen("PRETTY_HOSTNAME="));
+			break;
+		}
+	}
+
+	g_strfreev(lines);
+
+	return ret;
+}
+
+/*
+ * Device name expansion
+ *   %d - device id
+ *   %h - hostname
+ */
+static char *expand_name(char *dst, int size, char *str, int dev_id)
+{
+	register int sp, np, olen;
+	char *opt, buf[10];
+
+	if (!str || !dst)
+		return NULL;
+
+	sp = np = 0;
+	while (np < size - 1 && str[sp]) {
+		switch (str[sp]) {
+		case '%':
+			opt = NULL;
+
+			switch (str[sp+1]) {
+			case 'd':
+				sprintf(buf, "%d", dev_id);
+				opt = buf;
+				break;
+
+			case 'h':
+				opt = main_opts.host_name;
+				break;
+
+			case '%':
+				dst[np++] = str[sp++];
+				/* fall through */
+			default:
+				sp++;
+				continue;
+			}
+
+			if (opt) {
+				/* substitute */
+				olen = strlen(opt);
+				if (np + olen < size - 1)
+					memcpy(dst + np, opt, olen);
+				np += olen;
+			}
+			sp += 2;
+			continue;
+
+		case '\\':
+			sp++;
+			/* fall through */
+		default:
+			dst[np++] = str[sp++];
+			break;
+		}
+	}
+	dst[np] = '\0';
+	return dst;
+}
+
+static int
+get_default_adapter_id(void)
+{
+	struct btd_adapter *default_adapter;
+	default_adapter = manager_get_default_adapter();
+	if (default_adapter == NULL)
+		return -1;
+	return adapter_get_dev_id(default_adapter);
+}
+
+static int adaptername_probe(struct btd_adapter *adapter)
+{
+	int current_id;
+	char name[MAX_NAME_LENGTH + 1];
+	char *pretty_hostname;
+	bdaddr_t bdaddr;
+
+	current_id = adapter_get_dev_id(adapter);
+
+	pretty_hostname = read_pretty_host_name();
+	if (pretty_hostname != NULL) {
+		int default_adapter;
+
+		default_adapter = get_default_adapter_id();
+
+		/* Allow us to change the name */
+		adapter_set_allow_name_changes(adapter, TRUE);
+
+		/* If it's the first device, let's assume it will
+		 * be the default one, as we're not told when
+		 * the default adapter changes */
+		if (default_adapter < 0)
+			default_adapter = current_id;
+
+		if (default_adapter != current_id) {
+			char *str;
+
+			/* +1 because we don't want an adapter called "Foobar's laptop #0" */
+			str = g_strdup_printf("%s #%d", pretty_hostname, current_id + 1);
+			DBG("Setting name '%s' for device 'hci%d'", str, current_id);
+
+			adapter_update_local_name(adapter, str);
+			g_free(str);
+		} else {
+			DBG("Setting name '%s' for device 'hci%d'", pretty_hostname, current_id);
+			adapter_update_local_name(adapter, pretty_hostname);
+		}
+		g_free(pretty_hostname);
+
+		/* And disable the name change now */
+		adapter_set_allow_name_changes(adapter, FALSE);
+
+		return 0;
+	}
+
+	adapter_set_allow_name_changes(adapter, TRUE);
+	adapter_get_address(adapter, &bdaddr);
+
+	if (read_local_name(&bdaddr, name) < 0)
+		expand_name(name, MAX_NAME_LENGTH, main_opts.name,
+							current_id);
+	DBG("Setting name '%s' for device 'hci%d'", name, current_id);
+	adapter_update_local_name(adapter, name);
+
+	return 0;
+}
+
+static gboolean handle_inotify_cb(GIOChannel *channel,
+		GIOCondition condition, gpointer data)
+{
+	char buf[EVENT_BUF_LEN];
+	GIOStatus err;
+	gsize len, i;
+	gboolean changed;
+
+	changed = FALSE;
+
+	err = g_io_channel_read_chars(channel, buf, EVENT_BUF_LEN, &len, NULL);
+	if (err != G_IO_STATUS_NORMAL) {
+		error("Error reading inotify event: %d\n", err);
+		return FALSE;
+	}
+
+	i = 0;
+	while (i < len) {
+		struct inotify_event *pevent = (struct inotify_event *) &buf[i];
+
+		/* check that it's ours */
+		if (pevent->len &&
+		    pevent->name != NULL &&
+		    strcmp(pevent->name, MACHINE_INFO_FILE) == 0) {
+			changed = TRUE;
+		}
+		i += EVENT_SIZE + pevent->len;
+	}
+
+	if (changed != FALSE) {
+		DBG(MACHINE_INFO_DIR MACHINE_INFO_FILE
+				" changed, changing names for adapters");
+		manager_foreach_adapter((adapter_cb) adaptername_probe, NULL);
+	}
+
+	return TRUE;
+}
+
+static void adaptername_remove(struct btd_adapter *adapter)
+{
+	if (watch_fd >= 0)
+		close(watch_fd);
+	if (inotify != NULL)
+		g_io_channel_shutdown(inotify, FALSE, NULL);
+}
+
+static struct btd_adapter_driver adaptername_driver = {
+	.name	= "adaptername",
+	.probe	= adaptername_probe,
+	.remove	= adaptername_remove,
+};
+
+static int adaptername_init(void)
+{
+	int ret;
+	int inot_fd;
+
+	ret = btd_register_adapter_driver(&adaptername_driver);
+
+	if (ret != 0)
+		return ret;
+
+	inot_fd = inotify_init();
+	if (inot_fd < 0) {
+		error("Failed to setup inotify");
+		return 0;
+	}
+	watch_fd = inotify_add_watch(inot_fd,
+				     MACHINE_INFO_DIR,
+				     IN_CLOSE_WRITE | IN_DELETE | IN_CREATE);
+	if (watch_fd < 0) {
+		error("Failed to setup watch for '%s'",
+		      MACHINE_INFO_DIR);
+		return 0;
+	}
+
+	inotify = g_io_channel_unix_new(inot_fd);
+	g_io_channel_set_close_on_unref(inotify, TRUE);
+	g_io_channel_set_encoding(inotify, NULL, NULL);
+	g_io_channel_set_flags(inotify, G_IO_FLAG_NONBLOCK, NULL);
+	g_io_add_watch(inotify, G_IO_IN, handle_inotify_cb, NULL);
+
+	return 0;
+}
+
+static void adaptername_exit(void)
+{
+	btd_unregister_adapter_driver(&adaptername_driver);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(adaptername, VERSION,
+		BLUETOOTH_PLUGIN_PRIORITY_LOW, adaptername_init, adaptername_exit)
diff --git a/src/adapter.c b/src/adapter.c
index 5420acd..156d534 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -183,64 +183,6 @@ static void dev_info_free(struct remote_dev_info *dev)
 	g_free(dev);
 }
 
-/*
- * Device name expansion
- *   %d - device id
- */
-static char *expand_name(char *dst, int size, char *str, int dev_id)
-{
-	register int sp, np, olen;
-	char *opt, buf[10];
-
-	if (!str || !dst)
-		return NULL;
-
-	sp = np = 0;
-	while (np < size - 1 && str[sp]) {
-		switch (str[sp]) {
-		case '%':
-			opt = NULL;
-
-			switch (str[sp+1]) {
-			case 'd':
-				sprintf(buf, "%d", dev_id);
-				opt = buf;
-				break;
-
-			case 'h':
-				opt = main_opts.host_name;
-				break;
-
-			case '%':
-				dst[np++] = str[sp++];
-				/* fall through */
-			default:
-				sp++;
-				continue;
-			}
-
-			if (opt) {
-				/* substitute */
-				olen = strlen(opt);
-				if (np + olen < size - 1)
-					memcpy(dst + np, opt, olen);
-				np += olen;
-			}
-			sp += 2;
-			continue;
-
-		case '\\':
-			sp++;
-			/* fall through */
-		default:
-			dst[np++] = str[sp++];
-			break;
-		}
-	}
-	dst[np] = '\0';
-	return dst;
-}
-
 int btd_adapter_set_class(struct btd_adapter *adapter, uint8_t major,
 								uint8_t minor)
 {
@@ -2576,14 +2518,6 @@ gboolean adapter_init(struct btd_adapter *adapter)
 		return FALSE;
 	}
 
-	if (read_local_name(&adapter->bdaddr, adapter->name) < 0)
-		expand_name(adapter->name, MAX_NAME_LENGTH, main_opts.name,
-							adapter->dev_id);
-
-	if (main_opts.attrib_server)
-		attrib_gap_set(GATT_CHARAC_DEVICE_NAME,
-			(const uint8_t *) adapter->name, strlen(adapter->name));
-
 	sdp_init_services_list(&adapter->bdaddr);
 	load_drivers(adapter);
 	clear_blocked(adapter);
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* [PATCH] Add adaptername plugin
  2011-06-22 11:29 ` [PATCH 3/3] Add adaptername plugin Bastien Nocera
@ 2011-06-23 13:42   ` Bastien Nocera
  2011-06-23 13:44     ` Bastien Nocera
  2011-06-28  8:51     ` Johan Hedberg
  0 siblings, 2 replies; 9+ messages in thread
From: Bastien Nocera @ 2011-06-23 13:42 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Bastien Nocera

Replacing the name setting code in src/adapter.c.

Moving the adapter naming allows us to use the /etc/machine-info [1]
pretty hostname, as implemented by hostnamed [2] in systemd.

If /etc/machine-info is not present, the adapter name stored
on disk in /var/lib/bluetooth will be used. If no adapter name
has been set yet, the default from the main.conf will be used.

We don't currently number the name of hci0 if a pretty name is
available, but we should instead number it if it happens not
to be the default adapter. As we cannot be told when the default
adapter changes, we'll behave this way for now.

Note that when an adapter name is set automatically from
the pretty hostname, changing it through the D-Bus interface
will fail.

[1]: http://0pointer.de/public/systemd-man/machine-info.html
[2]: http://www.freedesktop.org/wiki/Software/systemd/hostnamed
---
 Makefile.am           |    3 +
 configure.ac          |    4 +
 plugins/adaptername.c |  306 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/adapter.c         |   66 -----------
 4 files changed, 313 insertions(+), 66 deletions(-)
 create mode 100644 plugins/adaptername.c

diff --git a/Makefile.am b/Makefile.am
index c2a6976..ba9a89d 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -218,6 +218,9 @@ EXTRA_DIST += plugins/hal.c plugins/formfactor.c
 builtin_modules += storage
 builtin_sources += plugins/storage.c
 
+builtin_modules += adaptername
+builtin_sources += plugins/adaptername.c
+
 if MAEMO6PLUGIN
 builtin_modules += maemo6
 builtin_sources += plugins/maemo6.c
diff --git a/configure.ac b/configure.ac
index 07b9e55..084ece5 100644
--- a/configure.ac
+++ b/configure.ac
@@ -35,6 +35,10 @@ AC_FUNC_PPOLL
 AC_CHECK_LIB(dl, dlopen, dummy=yes,
 			AC_MSG_ERROR(dynamic linking loader is required))
 
+AC_CHECK_HEADER([sys/inotify.h],
+		[AC_DEFINE([HAVE_SYS_INOTIFY_H], 1,
+			   [Define to 1 if you have <sys/inotify.h>.])],
+			   [AC_MSG_ERROR(inotify headers are required and missing)])
 AC_PATH_DBUS
 AC_PATH_GLIB
 AC_PATH_ALSA
diff --git a/plugins/adaptername.c b/plugins/adaptername.c
new file mode 100644
index 0000000..af4dac8
--- /dev/null
+++ b/plugins/adaptername.c
@@ -0,0 +1,306 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2011  Red Hat, Inc.
+ *
+ *
+ *  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
+ *
+ *  Author: Bastien Nocera <hadess@hadess.net>
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <bluetooth/bluetooth.h>
+
+#include "plugin.h"
+#include "hcid.h" /* For main_opts */
+#include "adapter.h"
+#include "manager.h"
+#include "device.h" /* Needed for storage.h */
+#include "storage.h"
+#include "log.h"
+
+#include <sys/inotify.h>
+#define EVENT_SIZE  (sizeof (struct inotify_event))
+#define EVENT_BUF_LEN (1024 * (EVENT_SIZE + 16))
+
+#define MACHINE_INFO_DIR "/etc/"
+#define MACHINE_INFO_FILE "machine-info"
+
+static GIOChannel *inotify = NULL;
+static int watch_fd = -1;
+
+/* This file is part of systemd's hostnamed functionality:
+ * http://0pointer.de/public/systemd-man/machine-info.html
+ * http://www.freedesktop.org/wiki/Software/systemd/hostnamed
+ */
+static char *read_pretty_host_name(void)
+{
+	char *contents, *ret;
+	char **lines;
+	guint i;
+
+	if (g_file_get_contents(MACHINE_INFO_DIR MACHINE_INFO_FILE,
+			&contents, NULL, NULL) == FALSE) {
+		return NULL;
+	}
+	lines = g_strsplit_set(contents, "\r\n", 0);
+	g_free(contents);
+
+	if (lines == NULL)
+		return NULL;
+
+	ret = NULL;
+	for (i = 0; lines[i] != NULL; i++) {
+		if (g_str_has_prefix(lines[i], "PRETTY_HOSTNAME=")) {
+			ret = g_strdup(lines[i] + strlen("PRETTY_HOSTNAME="));
+			break;
+		}
+	}
+
+	g_strfreev(lines);
+
+	return ret;
+}
+
+/*
+ * Device name expansion
+ *   %d - device id
+ *   %h - hostname
+ */
+static char *expand_name(char *dst, int size, char *str, int dev_id)
+{
+	register int sp, np, olen;
+	char *opt, buf[10];
+
+	if (!str || !dst)
+		return NULL;
+
+	sp = np = 0;
+	while (np < size - 1 && str[sp]) {
+		switch (str[sp]) {
+		case '%':
+			opt = NULL;
+
+			switch (str[sp+1]) {
+			case 'd':
+				sprintf(buf, "%d", dev_id);
+				opt = buf;
+				break;
+
+			case 'h':
+				opt = main_opts.host_name;
+				break;
+
+			case '%':
+				dst[np++] = str[sp++];
+				/* fall through */
+			default:
+				sp++;
+				continue;
+			}
+
+			if (opt) {
+				/* substitute */
+				olen = strlen(opt);
+				if (np + olen < size - 1)
+					memcpy(dst + np, opt, olen);
+				np += olen;
+			}
+			sp += 2;
+			continue;
+
+		case '\\':
+			sp++;
+			/* fall through */
+		default:
+			dst[np++] = str[sp++];
+			break;
+		}
+	}
+	dst[np] = '\0';
+	return dst;
+}
+
+static int
+get_default_adapter_id(void)
+{
+	struct btd_adapter *default_adapter;
+	default_adapter = manager_get_default_adapter();
+	if (default_adapter == NULL)
+		return -1;
+	return adapter_get_dev_id(default_adapter);
+}
+
+static int adaptername_probe(struct btd_adapter *adapter)
+{
+	int current_id;
+	char name[MAX_NAME_LENGTH + 1];
+	char *pretty_hostname;
+	bdaddr_t bdaddr;
+
+	current_id = adapter_get_dev_id(adapter);
+
+	pretty_hostname = read_pretty_host_name();
+	if (pretty_hostname != NULL) {
+		int default_adapter;
+
+		default_adapter = get_default_adapter_id();
+
+		/* Allow us to change the name */
+		adapter_set_allow_name_changes(adapter, TRUE);
+
+		/* If it's the first device, let's assume it will
+		 * be the default one, as we're not told when
+		 * the default adapter changes */
+		if (default_adapter < 0)
+			default_adapter = current_id;
+
+		if (default_adapter != current_id) {
+			char *str;
+
+			/* +1 because we don't want an adapter called "Foobar's laptop #0" */
+			str = g_strdup_printf("%s #%d", pretty_hostname, current_id + 1);
+			DBG("Setting name '%s' for device 'hci%d'", str, current_id);
+
+			adapter_update_local_name(adapter, str);
+			g_free(str);
+		} else {
+			DBG("Setting name '%s' for device 'hci%d'", pretty_hostname, current_id);
+			adapter_update_local_name(adapter, pretty_hostname);
+		}
+		g_free(pretty_hostname);
+
+		/* And disable the name change now */
+		adapter_set_allow_name_changes(adapter, FALSE);
+
+		return 0;
+	}
+
+	adapter_set_allow_name_changes(adapter, TRUE);
+	adapter_get_address(adapter, &bdaddr);
+
+	if (read_local_name(&bdaddr, name) < 0)
+		expand_name(name, MAX_NAME_LENGTH, main_opts.name,
+							current_id);
+	DBG("Setting name '%s' for device 'hci%d'", name, current_id);
+	adapter_update_local_name(adapter, name);
+
+	return 0;
+}
+
+static gboolean handle_inotify_cb(GIOChannel *channel,
+		GIOCondition condition, gpointer data)
+{
+	char buf[EVENT_BUF_LEN];
+	GIOStatus err;
+	gsize len, i;
+	gboolean changed;
+
+	changed = FALSE;
+
+	err = g_io_channel_read_chars(channel, buf, EVENT_BUF_LEN, &len, NULL);
+	if (err != G_IO_STATUS_NORMAL) {
+		error("Error reading inotify event: %d\n", err);
+		return FALSE;
+	}
+
+	i = 0;
+	while (i < len) {
+		struct inotify_event *pevent = (struct inotify_event *) &buf[i];
+
+		/* check that it's ours */
+		if (pevent->len &&
+		    pevent->name != NULL &&
+		    strcmp(pevent->name, MACHINE_INFO_FILE) == 0) {
+			changed = TRUE;
+		}
+		i += EVENT_SIZE + pevent->len;
+	}
+
+	if (changed != FALSE) {
+		DBG(MACHINE_INFO_DIR MACHINE_INFO_FILE
+				" changed, changing names for adapters");
+		manager_foreach_adapter((adapter_cb) adaptername_probe, NULL);
+	}
+
+	return TRUE;
+}
+
+static void adaptername_remove(struct btd_adapter *adapter)
+{
+	if (watch_fd >= 0)
+		close(watch_fd);
+	if (inotify != NULL)
+		g_io_channel_shutdown(inotify, FALSE, NULL);
+}
+
+static struct btd_adapter_driver adaptername_driver = {
+	.name	= "adaptername",
+	.probe	= adaptername_probe,
+	.remove	= adaptername_remove,
+};
+
+static int adaptername_init(void)
+{
+	int ret;
+	int inot_fd;
+
+	ret = btd_register_adapter_driver(&adaptername_driver);
+
+	if (ret != 0)
+		return ret;
+
+	inot_fd = inotify_init();
+	if (inot_fd < 0) {
+		error("Failed to setup inotify");
+		return 0;
+	}
+	watch_fd = inotify_add_watch(inot_fd,
+				     MACHINE_INFO_DIR,
+				     IN_CLOSE_WRITE | IN_DELETE | IN_CREATE |
+				     IN_MOVED_FROM | IN_MOVED_TO);
+	if (watch_fd < 0) {
+		error("Failed to setup watch for '%s'",
+		      MACHINE_INFO_DIR);
+		return 0;
+	}
+
+	inotify = g_io_channel_unix_new(inot_fd);
+	g_io_channel_set_close_on_unref(inotify, TRUE);
+	g_io_channel_set_encoding(inotify, NULL, NULL);
+	g_io_channel_set_flags(inotify, G_IO_FLAG_NONBLOCK, NULL);
+	g_io_add_watch(inotify, G_IO_IN, handle_inotify_cb, NULL);
+
+	return 0;
+}
+
+static void adaptername_exit(void)
+{
+	btd_unregister_adapter_driver(&adaptername_driver);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(adaptername, VERSION,
+		BLUETOOTH_PLUGIN_PRIORITY_LOW, adaptername_init, adaptername_exit)
diff --git a/src/adapter.c b/src/adapter.c
index 5420acd..156d534 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -183,64 +183,6 @@ static void dev_info_free(struct remote_dev_info *dev)
 	g_free(dev);
 }
 
-/*
- * Device name expansion
- *   %d - device id
- */
-static char *expand_name(char *dst, int size, char *str, int dev_id)
-{
-	register int sp, np, olen;
-	char *opt, buf[10];
-
-	if (!str || !dst)
-		return NULL;
-
-	sp = np = 0;
-	while (np < size - 1 && str[sp]) {
-		switch (str[sp]) {
-		case '%':
-			opt = NULL;
-
-			switch (str[sp+1]) {
-			case 'd':
-				sprintf(buf, "%d", dev_id);
-				opt = buf;
-				break;
-
-			case 'h':
-				opt = main_opts.host_name;
-				break;
-
-			case '%':
-				dst[np++] = str[sp++];
-				/* fall through */
-			default:
-				sp++;
-				continue;
-			}
-
-			if (opt) {
-				/* substitute */
-				olen = strlen(opt);
-				if (np + olen < size - 1)
-					memcpy(dst + np, opt, olen);
-				np += olen;
-			}
-			sp += 2;
-			continue;
-
-		case '\\':
-			sp++;
-			/* fall through */
-		default:
-			dst[np++] = str[sp++];
-			break;
-		}
-	}
-	dst[np] = '\0';
-	return dst;
-}
-
 int btd_adapter_set_class(struct btd_adapter *adapter, uint8_t major,
 								uint8_t minor)
 {
@@ -2576,14 +2518,6 @@ gboolean adapter_init(struct btd_adapter *adapter)
 		return FALSE;
 	}
 
-	if (read_local_name(&adapter->bdaddr, adapter->name) < 0)
-		expand_name(adapter->name, MAX_NAME_LENGTH, main_opts.name,
-							adapter->dev_id);
-
-	if (main_opts.attrib_server)
-		attrib_gap_set(GATT_CHARAC_DEVICE_NAME,
-			(const uint8_t *) adapter->name, strlen(adapter->name));
-
 	sdp_init_services_list(&adapter->bdaddr);
 	load_drivers(adapter);
 	clear_blocked(adapter);
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add adaptername plugin
  2011-06-23 13:42   ` [PATCH] " Bastien Nocera
@ 2011-06-23 13:44     ` Bastien Nocera
  2011-06-28  8:51     ` Johan Hedberg
  1 sibling, 0 replies; 9+ messages in thread
From: Bastien Nocera @ 2011-06-23 13:44 UTC (permalink / raw)
  To: linux-bluetooth

On Thu, 2011-06-23 at 14:42 +0100, Bastien Nocera wrote:
> Replacing the name setting code in src/adapter.c.

This new patch corrects the copyright of the plugin, and makes inotify
look for a few more events (so that you can test the code my
renaming /etc/machine-info to a .bak or something).


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add adaptername plugin
  2011-06-23 13:42   ` [PATCH] " Bastien Nocera
  2011-06-23 13:44     ` Bastien Nocera
@ 2011-06-28  8:51     ` Johan Hedberg
  2011-06-28  9:21       ` Bastien Nocera
  2011-06-28  9:21       ` Bastien Nocera
  1 sibling, 2 replies; 9+ messages in thread
From: Johan Hedberg @ 2011-06-28  8:51 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: linux-bluetooth

Hi Bastien,

I've pushed the two first patches and was also about to push this one
after doing some minor fixes first myself. However, I kept on finding
more and more issues to fix, so unfortunately here goes another feedback
round...

On Thu, Jun 23, 2011, Bastien Nocera wrote:
> + *  BlueZ - Bluetooth protocol stack for Linux
> + *
> + *  Copyright (C) 2011  Red Hat, Inc.
> + *

You'll still need to keep also Marcel's copyright here due to
considerable portions (such as the expand_name function) having been
copied from elsewhere.

> +static int adaptername_probe(struct btd_adapter *adapter)
> +{
> +	int current_id;
> +	char name[MAX_NAME_LENGTH + 1];
> +	char *pretty_hostname;
> +	bdaddr_t bdaddr;
> +
> +	current_id = adapter_get_dev_id(adapter);
> +
> +	pretty_hostname = read_pretty_host_name();
> +	if (pretty_hostname != NULL) {
> +		int default_adapter;
> +
> +		default_adapter = get_default_adapter_id();
> +
> +		/* Allow us to change the name */
> +		adapter_set_allow_name_changes(adapter, TRUE);
> +
> +		/* If it's the first device, let's assume it will
> +		 * be the default one, as we're not told when
> +		 * the default adapter changes */
> +		if (default_adapter < 0)
> +			default_adapter = current_id;
> +
> +		if (default_adapter != current_id) {
> +			char *str;
> +
> +			/* +1 because we don't want an adapter called "Foobar's laptop #0" */
> +			str = g_strdup_printf("%s #%d", pretty_hostname, current_id + 1);
> +			DBG("Setting name '%s' for device 'hci%d'", str, current_id);
> +
> +			adapter_update_local_name(adapter, str);
> +			g_free(str);
> +		} else {
> +			DBG("Setting name '%s' for device 'hci%d'", pretty_hostname, current_id);
> +			adapter_update_local_name(adapter, pretty_hostname);
> +		}
> +		g_free(pretty_hostname);
> +
> +		/* And disable the name change now */
> +		adapter_set_allow_name_changes(adapter, FALSE);
> +
> +		return 0;
> +	}

I think I already mentioned this in another email, but this longish
if-branch should really be its own function. E.g. something like:

	if (pretty_hostname != NULL) {
		set_pretty_host_name(adapter, pretty_hostname);
		return 0;
	}

> +	while (i < len) {
> +		struct inotify_event *pevent = (struct inotify_event *) &buf[i];
> +
> +		/* check that it's ours */
> +		if (pevent->len &&
> +		    pevent->name != NULL &&
> +		    strcmp(pevent->name, MACHINE_INFO_FILE) == 0) {

The above two lines mix tabs and spaces for indentation.

> +static void adaptername_remove(struct btd_adapter *adapter)
> +{
> +	if (watch_fd >= 0)
> +		close(watch_fd);
> +	if (inotify != NULL)
> +		g_io_channel_shutdown(inotify, FALSE, NULL);
> +}
> +
> +static struct btd_adapter_driver adaptername_driver = {
> +	.name	= "adaptername",
> +	.probe	= adaptername_probe,
> +	.remove	= adaptername_remove,
> +};
> +
> +static int adaptername_init(void)
> +{
> +	int ret;
> +	int inot_fd;
> +
> +	ret = btd_register_adapter_driver(&adaptername_driver);
> +
> +	if (ret != 0)
> +		return ret;

Please use "err" and the check should be if (err < 0). Also, no empty
line before the if-statement.

> +	inot_fd = inotify_init();
> +	if (inot_fd < 0) {
> +		error("Failed to setup inotify");
> +		return 0;
> +	}
> +	watch_fd = inotify_add_watch(inot_fd,

Empty line after after the } line please.

> +				     MACHINE_INFO_DIR,
> +				     IN_CLOSE_WRITE | IN_DELETE | IN_CREATE |
> +				     IN_MOVED_FROM | IN_MOVED_TO);

Mixed tabs and spaces in the above lines. The code might get easier to
read if you created a "uint32_t mask" variable in the function and
passed that to the inotify_add_watch call.

> +	if (watch_fd < 0) {
> +		error("Failed to setup watch for '%s'",
> +		      MACHINE_INFO_DIR);

Mixed tabs and spaces in the above line.

> +		return 0;
> +	}

Shouldn't you close(inot_fd) before the above return?

Johan

^ permalink raw reply	[flat|nested] 9+ messages in thread

* [PATCH] Add adaptername plugin
  2011-06-28  8:51     ` Johan Hedberg
@ 2011-06-28  9:21       ` Bastien Nocera
  2011-06-28 10:23         ` Johan Hedberg
  2011-06-28  9:21       ` Bastien Nocera
  1 sibling, 1 reply; 9+ messages in thread
From: Bastien Nocera @ 2011-06-28  9:21 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: johan.hedberg, Bastien Nocera

Replacing the name setting code in src/adapter.c.

Moving the adapter naming allows us to use the /etc/machine-info [1]
pretty hostname, as implemented by hostnamed [2] in systemd.

If /etc/machine-info is not present, the adapter name stored
on disk in /var/lib/bluetooth will be used. If no adapter name
has been set yet, the default from the main.conf will be used.

We don't currently number the name of hci0 if a pretty name is
available, but we should instead number it if it happens not
to be the default adapter. As we cannot be told when the default
adapter changes, we'll behave this way for now.

Note that when an adapter name is set automatically from
the pretty hostname, changing it through the D-Bus interface
will fail.

[1]: http://0pointer.de/public/systemd-man/machine-info.html
[2]: http://www.freedesktop.org/wiki/Software/systemd/hostnamed
---
 Makefile.am           |    3 +
 configure.ac          |    4 +
 plugins/adaptername.c |  321 +++++++++++++++++++++++++++++++++++++++++++++++++
 src/adapter.c         |   66 ----------
 4 files changed, 328 insertions(+), 66 deletions(-)
 create mode 100644 plugins/adaptername.c

diff --git a/Makefile.am b/Makefile.am
index 4659c80..e320105 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -218,6 +218,9 @@ EXTRA_DIST += plugins/hal.c plugins/formfactor.c
 builtin_modules += storage
 builtin_sources += plugins/storage.c
 
+builtin_modules += adaptername
+builtin_sources += plugins/adaptername.c
+
 if MAEMO6PLUGIN
 builtin_modules += maemo6
 builtin_sources += plugins/maemo6.c
diff --git a/configure.ac b/configure.ac
index 2ce4acf..5f029fb 100644
--- a/configure.ac
+++ b/configure.ac
@@ -35,6 +35,10 @@ AC_FUNC_PPOLL
 AC_CHECK_LIB(dl, dlopen, dummy=yes,
 			AC_MSG_ERROR(dynamic linking loader is required))
 
+AC_CHECK_HEADER([sys/inotify.h],
+		[AC_DEFINE([HAVE_SYS_INOTIFY_H], 1,
+			   [Define to 1 if you have <sys/inotify.h>.])],
+			   [AC_MSG_ERROR(inotify headers are required and missing)])
 AC_PATH_DBUS
 AC_PATH_GLIB
 AC_PATH_ALSA
diff --git a/plugins/adaptername.c b/plugins/adaptername.c
new file mode 100644
index 0000000..23f87c5
--- /dev/null
+++ b/plugins/adaptername.c
@@ -0,0 +1,321 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2011  Red Hat, Inc.
+ *  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
+ *
+ *  Author: Bastien Nocera <hadess@hadess.net>
+ *  Marcel Holtmann <marcel@holtmann.org> (for expand_name)
+ *
+ */
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#include <stdlib.h>
+#include <unistd.h>
+#include <errno.h>
+
+#include <glib.h>
+#include <bluetooth/bluetooth.h>
+
+#include "plugin.h"
+#include "hcid.h" /* For main_opts */
+#include "adapter.h"
+#include "manager.h"
+#include "device.h" /* Needed for storage.h */
+#include "storage.h"
+#include "log.h"
+
+#include <sys/inotify.h>
+#define EVENT_SIZE  (sizeof (struct inotify_event))
+#define EVENT_BUF_LEN (1024 * (EVENT_SIZE + 16))
+
+#define MACHINE_INFO_DIR "/etc/"
+#define MACHINE_INFO_FILE "machine-info"
+
+static GIOChannel *inotify = NULL;
+static int watch_fd = -1;
+
+/* This file is part of systemd's hostnamed functionality:
+ * http://0pointer.de/public/systemd-man/machine-info.html
+ * http://www.freedesktop.org/wiki/Software/systemd/hostnamed
+ */
+static char *read_pretty_host_name(void)
+{
+	char *contents, *ret;
+	char **lines;
+	guint i;
+
+	if (g_file_get_contents(MACHINE_INFO_DIR MACHINE_INFO_FILE,
+			&contents, NULL, NULL) == FALSE) {
+		return NULL;
+	}
+	lines = g_strsplit_set(contents, "\r\n", 0);
+	g_free(contents);
+
+	if (lines == NULL)
+		return NULL;
+
+	ret = NULL;
+	for (i = 0; lines[i] != NULL; i++) {
+		if (g_str_has_prefix(lines[i], "PRETTY_HOSTNAME=")) {
+			ret = g_strdup(lines[i] + strlen("PRETTY_HOSTNAME="));
+			break;
+		}
+	}
+
+	g_strfreev(lines);
+
+	return ret;
+}
+
+/*
+ * Device name expansion
+ *   %d - device id
+ *   %h - hostname
+ */
+static char *expand_name(char *dst, int size, char *str, int dev_id)
+{
+	register int sp, np, olen;
+	char *opt, buf[10];
+
+	if (!str || !dst)
+		return NULL;
+
+	sp = np = 0;
+	while (np < size - 1 && str[sp]) {
+		switch (str[sp]) {
+		case '%':
+			opt = NULL;
+
+			switch (str[sp+1]) {
+			case 'd':
+				sprintf(buf, "%d", dev_id);
+				opt = buf;
+				break;
+
+			case 'h':
+				opt = main_opts.host_name;
+				break;
+
+			case '%':
+				dst[np++] = str[sp++];
+				/* fall through */
+			default:
+				sp++;
+				continue;
+			}
+
+			if (opt) {
+				/* substitute */
+				olen = strlen(opt);
+				if (np + olen < size - 1)
+					memcpy(dst + np, opt, olen);
+				np += olen;
+			}
+			sp += 2;
+			continue;
+
+		case '\\':
+			sp++;
+			/* fall through */
+		default:
+			dst[np++] = str[sp++];
+			break;
+		}
+	}
+	dst[np] = '\0';
+	return dst;
+}
+
+static int
+get_default_adapter_id(void)
+{
+	struct btd_adapter *default_adapter;
+	default_adapter = manager_get_default_adapter();
+	if (default_adapter == NULL)
+		return -1;
+	return adapter_get_dev_id(default_adapter);
+}
+
+static void
+set_pretty_name(struct btd_adapter *adapter,
+		const char *pretty_hostname)
+{
+	int current_id;
+	int default_adapter;
+
+	default_adapter = get_default_adapter_id();
+	current_id = adapter_get_dev_id(adapter);
+
+	/* Allow us to change the name */
+	adapter_set_allow_name_changes(adapter, TRUE);
+
+	/* If it's the first device, let's assume it will
+	 * be the default one, as we're not told when
+	 * the default adapter changes */
+	if (default_adapter < 0)
+		default_adapter = current_id;
+
+	if (default_adapter != current_id) {
+		char *str;
+
+		/* +1 because we don't want an adapter called "Foobar's laptop #0" */
+		str = g_strdup_printf("%s #%d", pretty_hostname, current_id + 1);
+		DBG("Setting name '%s' for device 'hci%d'", str, current_id);
+
+		adapter_update_local_name(adapter, str);
+		g_free(str);
+	} else {
+		DBG("Setting name '%s' for device 'hci%d'", pretty_hostname, current_id);
+		adapter_update_local_name(adapter, pretty_hostname);
+	}
+
+	/* And disable the name change now */
+	adapter_set_allow_name_changes(adapter, FALSE);
+}
+
+static int adaptername_probe(struct btd_adapter *adapter)
+{
+	int current_id;
+	char name[MAX_NAME_LENGTH + 1];
+	char *pretty_hostname;
+	bdaddr_t bdaddr;
+
+	pretty_hostname = read_pretty_host_name();
+	if (pretty_hostname != NULL) {
+		set_pretty_name(adapter, pretty_hostname);
+		g_free(pretty_hostname);
+		return 0;
+	}
+
+	adapter_set_allow_name_changes(adapter, TRUE);
+	adapter_get_address(adapter, &bdaddr);
+	current_id = adapter_get_dev_id(adapter);
+
+	if (read_local_name(&bdaddr, name) < 0)
+		expand_name(name, MAX_NAME_LENGTH, main_opts.name,
+							current_id);
+	DBG("Setting name '%s' for device 'hci%d'", name, current_id);
+	adapter_update_local_name(adapter, name);
+
+	return 0;
+}
+
+static gboolean handle_inotify_cb(GIOChannel *channel,
+		GIOCondition condition, gpointer data)
+{
+	char buf[EVENT_BUF_LEN];
+	GIOStatus err;
+	gsize len, i;
+	gboolean changed;
+
+	changed = FALSE;
+
+	err = g_io_channel_read_chars(channel, buf, EVENT_BUF_LEN, &len, NULL);
+	if (err != G_IO_STATUS_NORMAL) {
+		error("Error reading inotify event: %d\n", err);
+		return FALSE;
+	}
+
+	i = 0;
+	while (i < len) {
+		struct inotify_event *pevent = (struct inotify_event *) &buf[i];
+
+		/* check that it's ours */
+		if (pevent->len &&
+			pevent->name != NULL &&
+			strcmp(pevent->name, MACHINE_INFO_FILE) == 0) {
+			changed = TRUE;
+		}
+		i += EVENT_SIZE + pevent->len;
+	}
+
+	if (changed != FALSE) {
+		DBG(MACHINE_INFO_DIR MACHINE_INFO_FILE
+				" changed, changing names for adapters");
+		manager_foreach_adapter((adapter_cb) adaptername_probe, NULL);
+	}
+
+	return TRUE;
+}
+
+static void adaptername_remove(struct btd_adapter *adapter)
+{
+	if (watch_fd >= 0)
+		close(watch_fd);
+	if (inotify != NULL)
+		g_io_channel_shutdown(inotify, FALSE, NULL);
+}
+
+static struct btd_adapter_driver adaptername_driver = {
+	.name	= "adaptername",
+	.probe	= adaptername_probe,
+	.remove	= adaptername_remove,
+};
+
+static int adaptername_init(void)
+{
+	int err;
+	int inot_fd;
+	guint32 mask;
+
+	err = btd_register_adapter_driver(&adaptername_driver);
+
+	if (err < 0)
+		return err;
+
+	inot_fd = inotify_init();
+	if (inot_fd < 0) {
+		error("Failed to setup inotify");
+		return 0;
+	}
+
+	mask = IN_CLOSE_WRITE;
+	mask |= IN_DELETE;
+	mask |= IN_CREATE;
+	mask |= IN_MOVED_FROM;
+	mask |= IN_MOVED_TO;
+
+	watch_fd = inotify_add_watch(inot_fd, MACHINE_INFO_DIR, mask);
+	if (watch_fd < 0) {
+		error("Failed to setup watch for '%s'",
+			MACHINE_INFO_DIR);
+		close(inot_fd);
+		return 0;
+	}
+
+	inotify = g_io_channel_unix_new(inot_fd);
+	g_io_channel_set_close_on_unref(inotify, TRUE);
+	g_io_channel_set_encoding(inotify, NULL, NULL);
+	g_io_channel_set_flags(inotify, G_IO_FLAG_NONBLOCK, NULL);
+	g_io_add_watch(inotify, G_IO_IN, handle_inotify_cb, NULL);
+
+	return 0;
+}
+
+static void adaptername_exit(void)
+{
+	btd_unregister_adapter_driver(&adaptername_driver);
+}
+
+BLUETOOTH_PLUGIN_DEFINE(adaptername, VERSION,
+		BLUETOOTH_PLUGIN_PRIORITY_LOW, adaptername_init, adaptername_exit)
diff --git a/src/adapter.c b/src/adapter.c
index 2a10bc2..99d3eee 100644
--- a/src/adapter.c
+++ b/src/adapter.c
@@ -183,64 +183,6 @@ static void dev_info_free(struct remote_dev_info *dev)
 	g_free(dev);
 }
 
-/*
- * Device name expansion
- *   %d - device id
- */
-static char *expand_name(char *dst, int size, char *str, int dev_id)
-{
-	register int sp, np, olen;
-	char *opt, buf[10];
-
-	if (!str || !dst)
-		return NULL;
-
-	sp = np = 0;
-	while (np < size - 1 && str[sp]) {
-		switch (str[sp]) {
-		case '%':
-			opt = NULL;
-
-			switch (str[sp+1]) {
-			case 'd':
-				sprintf(buf, "%d", dev_id);
-				opt = buf;
-				break;
-
-			case 'h':
-				opt = main_opts.host_name;
-				break;
-
-			case '%':
-				dst[np++] = str[sp++];
-				/* fall through */
-			default:
-				sp++;
-				continue;
-			}
-
-			if (opt) {
-				/* substitute */
-				olen = strlen(opt);
-				if (np + olen < size - 1)
-					memcpy(dst + np, opt, olen);
-				np += olen;
-			}
-			sp += 2;
-			continue;
-
-		case '\\':
-			sp++;
-			/* fall through */
-		default:
-			dst[np++] = str[sp++];
-			break;
-		}
-	}
-	dst[np] = '\0';
-	return dst;
-}
-
 int btd_adapter_set_class(struct btd_adapter *adapter, uint8_t major,
 								uint8_t minor)
 {
@@ -2576,14 +2518,6 @@ gboolean adapter_init(struct btd_adapter *adapter)
 		return FALSE;
 	}
 
-	if (read_local_name(&adapter->bdaddr, adapter->name) < 0)
-		expand_name(adapter->name, MAX_NAME_LENGTH, main_opts.name,
-							adapter->dev_id);
-
-	if (main_opts.attrib_server)
-		attrib_gap_set(GATT_CHARAC_DEVICE_NAME,
-			(const uint8_t *) adapter->name, strlen(adapter->name));
-
 	sdp_init_services_list(&adapter->bdaddr);
 	load_drivers(adapter);
 	clear_blocked(adapter);
-- 
1.7.5.4


^ permalink raw reply related	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add adaptername plugin
  2011-06-28  8:51     ` Johan Hedberg
  2011-06-28  9:21       ` Bastien Nocera
@ 2011-06-28  9:21       ` Bastien Nocera
  1 sibling, 0 replies; 9+ messages in thread
From: Bastien Nocera @ 2011-06-28  9:21 UTC (permalink / raw)
  To: Johan Hedberg; +Cc: linux-bluetooth

On Tue, 2011-06-28 at 11:51 +0300, Johan Hedberg wrote:
> Hi Bastien,
> 
> I've pushed the two first patches and was also about to push this one
> after doing some minor fixes first myself. However, I kept on finding
> more and more issues to fix, so unfortunately here goes another feedback
> round...
> 
> On Thu, Jun 23, 2011, Bastien Nocera wrote:
> > + *  BlueZ - Bluetooth protocol stack for Linux
> > + *
> > + *  Copyright (C) 2011  Red Hat, Inc.
> > + *
> 
> You'll still need to keep also Marcel's copyright here due to
> considerable portions (such as the expand_name function) having been
> copied from elsewhere.

Done.

> > +static int adaptername_probe(struct btd_adapter *adapter)
> > +{
> > +	int current_id;
> > +	char name[MAX_NAME_LENGTH + 1];
> > +	char *pretty_hostname;
> > +	bdaddr_t bdaddr;
> > +
> > +	current_id = adapter_get_dev_id(adapter);
> > +
> > +	pretty_hostname = read_pretty_host_name();
> > +	if (pretty_hostname != NULL) {
> > +		int default_adapter;
> > +
> > +		default_adapter = get_default_adapter_id();
> > +
> > +		/* Allow us to change the name */
> > +		adapter_set_allow_name_changes(adapter, TRUE);
> > +
> > +		/* If it's the first device, let's assume it will
> > +		 * be the default one, as we're not told when
> > +		 * the default adapter changes */
> > +		if (default_adapter < 0)
> > +			default_adapter = current_id;
> > +
> > +		if (default_adapter != current_id) {
> > +			char *str;
> > +
> > +			/* +1 because we don't want an adapter called "Foobar's laptop #0" */
> > +			str = g_strdup_printf("%s #%d", pretty_hostname, current_id + 1);
> > +			DBG("Setting name '%s' for device 'hci%d'", str, current_id);
> > +
> > +			adapter_update_local_name(adapter, str);
> > +			g_free(str);
> > +		} else {
> > +			DBG("Setting name '%s' for device 'hci%d'", pretty_hostname, current_id);
> > +			adapter_update_local_name(adapter, pretty_hostname);
> > +		}
> > +		g_free(pretty_hostname);
> > +
> > +		/* And disable the name change now */
> > +		adapter_set_allow_name_changes(adapter, FALSE);
> > +
> > +		return 0;
> > +	}
> 
> I think I already mentioned this in another email, but this longish
> if-branch should really be its own function. E.g. something like:

Done.

> 	if (pretty_hostname != NULL) {
> 		set_pretty_host_name(adapter, pretty_hostname);
> 		return 0;
> 	}
> 
> > +	while (i < len) {
> > +		struct inotify_event *pevent = (struct inotify_event *) &buf[i];
> > +
> > +		/* check that it's ours */
> > +		if (pevent->len &&
> > +		    pevent->name != NULL &&
> > +		    strcmp(pevent->name, MACHINE_INFO_FILE) == 0) {
> 
> The above two lines mix tabs and spaces for indentation.

Fixed.

> > +static void adaptername_remove(struct btd_adapter *adapter)
> > +{
> > +	if (watch_fd >= 0)
> > +		close(watch_fd);
> > +	if (inotify != NULL)
> > +		g_io_channel_shutdown(inotify, FALSE, NULL);
> > +}
> > +
> > +static struct btd_adapter_driver adaptername_driver = {
> > +	.name	= "adaptername",
> > +	.probe	= adaptername_probe,
> > +	.remove	= adaptername_remove,
> > +};
> > +
> > +static int adaptername_init(void)
> > +{
> > +	int ret;
> > +	int inot_fd;
> > +
> > +	ret = btd_register_adapter_driver(&adaptername_driver);
> > +
> > +	if (ret != 0)
> > +		return ret;
> 
> Please use "err" and the check should be if (err < 0). Also, no empty
> line before the if-statement.

Fixed.

> > +	inot_fd = inotify_init();
> > +	if (inot_fd < 0) {
> > +		error("Failed to setup inotify");
> > +		return 0;
> > +	}
> > +	watch_fd = inotify_add_watch(inot_fd,
> 
> Empty line after after the } line please.

done.

> > +				     MACHINE_INFO_DIR,
> > +				     IN_CLOSE_WRITE | IN_DELETE | IN_CREATE |
> > +				     IN_MOVED_FROM | IN_MOVED_TO);
> 
> Mixed tabs and spaces in the above lines. The code might get easier to
> read if you created a "uint32_t mask" variable in the function and
> passed that to the inotify_add_watch call.

Done.

> > +	if (watch_fd < 0) {
> > +		error("Failed to setup watch for '%s'",
> > +		      MACHINE_INFO_DIR);
> 
> Mixed tabs and spaces in the above line.

Done.

> > +		return 0;
> > +	}
> 
> Shouldn't you close(inot_fd) before the above return?

Done.


^ permalink raw reply	[flat|nested] 9+ messages in thread

* Re: [PATCH] Add adaptername plugin
  2011-06-28  9:21       ` Bastien Nocera
@ 2011-06-28 10:23         ` Johan Hedberg
  0 siblings, 0 replies; 9+ messages in thread
From: Johan Hedberg @ 2011-06-28 10:23 UTC (permalink / raw)
  To: Bastien Nocera; +Cc: linux-bluetooth

Hi Bastien,

On Tue, Jun 28, 2011, Bastien Nocera wrote:
> Replacing the name setting code in src/adapter.c.
> 
> Moving the adapter naming allows us to use the /etc/machine-info [1]
> pretty hostname, as implemented by hostnamed [2] in systemd.
> 
> If /etc/machine-info is not present, the adapter name stored
> on disk in /var/lib/bluetooth will be used. If no adapter name
> has been set yet, the default from the main.conf will be used.
> 
> We don't currently number the name of hci0 if a pretty name is
> available, but we should instead number it if it happens not
> to be the default adapter. As we cannot be told when the default
> adapter changes, we'll behave this way for now.
> 
> Note that when an adapter name is set automatically from
> the pretty hostname, changing it through the D-Bus interface
> will fail.
> 
> [1]: http://0pointer.de/public/systemd-man/machine-info.html
> [2]: http://www.freedesktop.org/wiki/Software/systemd/hostnamed
> ---
>  Makefile.am           |    3 +
>  configure.ac          |    4 +
>  plugins/adaptername.c |  321 +++++++++++++++++++++++++++++++++++++++++++++++++
>  src/adapter.c         |   66 ----------
>  4 files changed, 328 insertions(+), 66 deletions(-)
>  create mode 100644 plugins/adaptername.c

Thanks for the quick response. After a couple of more (minor) coding
style fixes the patch is now upstream.

Johan

^ permalink raw reply	[flat|nested] 9+ messages in thread

end of thread, other threads:[~2011-06-28 10:23 UTC | newest]

Thread overview: 9+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2011-06-22 11:29 [PATCH 1/3] Simplify "SetName" D-Bus call Bastien Nocera
2011-06-22 11:29 ` [PATCH 2/3] Add ability to block name changes Bastien Nocera
2011-06-22 11:29 ` [PATCH 3/3] Add adaptername plugin Bastien Nocera
2011-06-23 13:42   ` [PATCH] " Bastien Nocera
2011-06-23 13:44     ` Bastien Nocera
2011-06-28  8:51     ` Johan Hedberg
2011-06-28  9:21       ` Bastien Nocera
2011-06-28 10:23         ` Johan Hedberg
2011-06-28  9:21       ` Bastien Nocera

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).