All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH 1/5] serial: add Serial.ConnectFD()
@ 2011-08-18 20:09 Gustavo F. Padovan
  2011-08-18 20:10 ` [PATCH 2/5] serial: fix unref of dbus message Gustavo F. Padovan
  2011-08-19  7:07 ` [PATCH 1/5] serial: add Serial.ConnectFD() Luiz Augusto von Dentz
  0 siblings, 2 replies; 11+ messages in thread
From: Gustavo F. Padovan @ 2011-08-18 20:09 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo F. Padovan

From: "Gustavo F. Padovan" <padovan@profusion.mobi>

It's similar to Serial.Connect() but returns the actual RFCOMM file
descriptor instead of creating a device in /dev
---
 doc/serial-api.txt |   16 ++++++++++++++++
 serial/port.c      |   36 ++++++++++++++++++++++++++++++++++--
 2 files changed, 50 insertions(+), 2 deletions(-)

diff --git a/doc/serial-api.txt b/doc/serial-api.txt
index 5f9bd5f..635b3ec 100644
--- a/doc/serial-api.txt
+++ b/doc/serial-api.txt
@@ -26,6 +26,22 @@ Methods		string Connect(string pattern)
 					 org.bluez.Error.ConnectionAttemptFailed
 					 org.bluez.Error.NotSupported
 
+Methods		fd ConnectFD(string pattern)
+
+			Connects to a specific RFCOMM based service on a
+			remote device and returns a file descriptor to talk
+			 with this device.
+
+			Possible patterns: UUID 128 bit as string
+					   Profile short names, e.g: spp, dun
+					   RFCOMM channel as string, 1-30
+
+			Possible errors: org.bluez.Error.InvalidArguments
+					 org.bluez.Error.InProgress
+					 org.bluez.Error.ConnectionAttemptFailed
+					 org.bluez.Error.NotSupported
+
+
 		void Disconnect(string device)
 
 			Disconnect a RFCOMM TTY device that has been
diff --git a/serial/port.c b/serial/port.c
index d011084..e359716 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -63,6 +63,10 @@
 #define MAX_OPEN_TRIES		5
 #define OPEN_WAIT		300	/* ms. udev node creation retry wait */
 
+#ifndef DBUS_TYPE_UNIX_FD
+#define DBUS_TYPE_UNIX_FD -1
+#endif
+
 struct serial_device {
 	DBusConnection	*conn;		/* for name listener handling */
 	bdaddr_t	src;		/* Source (local) address */
@@ -80,6 +84,7 @@ struct serial_port {
 	int		fd;		/* Opened file descriptor */
 	GIOChannel	*io;		/* BtIO channel */
 	guint		listener_id;
+	gboolean	fd_passing;
 	struct serial_device *device;
 };
 
@@ -329,6 +334,14 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *conn_err,
 	port->io = NULL;
 
 	sk = g_io_channel_unix_get_fd(chan);
+	if (port->fd_passing) {
+		reply = g_dbus_create_reply(port->msg,
+				DBUS_TYPE_UNIX_FD, &sk,
+				DBUS_TYPE_INVALID);
+		g_dbus_send_message(device->conn, reply);
+		return;
+	}
+
 	port->id = ioctl(sk, RFCOMMCREATEDEV, &req);
 	if (port->id < 0) {
 		int err = -errno;
@@ -468,8 +481,8 @@ static struct serial_port *create_port(struct serial_device *device,
 	return port;
 }
 
-static DBusMessage *port_connect(DBusConnection *conn,
-					DBusMessage *msg, void *user_data)
+static DBusMessage *_port_connect(DBusConnection *conn, DBusMessage *msg,
+					void *user_data, gboolean fd)
 {
 	struct serial_device *device = user_data;
 	struct serial_port *port;
@@ -501,6 +514,9 @@ static DBusMessage *port_connect(DBusConnection *conn,
 						NULL);
 	port->msg = dbus_message_ref(msg);
 
+	if (fd)
+		port->fd_passing = TRUE;
+
 	err = connect_port(port);
 	if (err < 0) {
 		error("%s", strerror(-err));
@@ -513,6 +529,21 @@ static DBusMessage *port_connect(DBusConnection *conn,
 	return NULL;
 }
 
+static DBusMessage *port_connect(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	return _port_connect(conn, msg, user_data, FALSE);
+}
+
+static DBusMessage *port_connect_fd(DBusConnection *conn,
+					DBusMessage *msg, void *user_data)
+{
+	if (DBUS_TYPE_UNIX_FD < 0)
+		return btd_error_not_supported(msg);
+
+	return _port_connect(conn, msg, user_data, TRUE);
+}
+
 static DBusMessage *port_disconnect(DBusConnection *conn,
 					DBusMessage *msg, void *user_data)
 {
@@ -546,6 +577,7 @@ static DBusMessage *port_disconnect(DBusConnection *conn,
 
 static GDBusMethodTable port_methods[] = {
 	{ "Connect",    "s", "s", port_connect, G_DBUS_METHOD_FLAG_ASYNC },
+	{ "ConnectFD",    "s", "h", port_connect_fd, G_DBUS_METHOD_FLAG_ASYNC },
 	{ "Disconnect", "s", "",  port_disconnect },
 	{ }
 };
-- 
1.7.6


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

* [PATCH 2/5] serial: fix unref of dbus message
  2011-08-18 20:09 [PATCH 1/5] serial: add Serial.ConnectFD() Gustavo F. Padovan
@ 2011-08-18 20:10 ` Gustavo F. Padovan
  2011-08-18 20:10   ` [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure Gustavo F. Padovan
  2011-08-19  7:07 ` [PATCH 1/5] serial: add Serial.ConnectFD() Luiz Augusto von Dentz
  1 sibling, 1 reply; 11+ messages in thread
From: Gustavo F. Padovan @ 2011-08-18 20:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo F. Padovan

From: "Gustavo F. Padovan" <padovan@profusion.mobi>

port->msg needs unref when the reply is sent.
---
 serial/port.c |   18 ++++++++++++++++++
 1 files changed, 18 insertions(+), 0 deletions(-)

diff --git a/serial/port.c b/serial/port.c
index e359716..3f041a6 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -204,6 +204,11 @@ static void serial_port_free(void *data)
 
 	port_release(port);
 
+	if (port->msg) {
+		dbus_message_unref(port->msg);
+		port->msg = NULL;
+	}
+
 	g_free(port->uuid);
 	g_free(port);
 }
@@ -224,6 +229,11 @@ static void port_owner_exited(DBusConnection *conn, void *user_data)
 
 	port_release(port);
 
+	if (port->msg) {
+		dbus_message_unref(port->msg);
+		port->msg = NULL;
+	}
+
 	port->listener_id = 0;
 }
 
@@ -261,6 +271,8 @@ static void open_notify(int fd, int err, struct serial_port *port)
 
 	/* Reply to the requestor */
 	g_dbus_send_message(device->conn, reply);
+	dbus_message_unref(port->msg);
+	port->msg = NULL;
 }
 
 static gboolean open_continue(gpointer user_data)
@@ -339,6 +351,8 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *conn_err,
 				DBUS_TYPE_UNIX_FD, &sk,
 				DBUS_TYPE_INVALID);
 		g_dbus_send_message(device->conn, reply);
+		dbus_message_unref(port->msg);
+		port->msg = NULL;
 		return;
 	}
 
@@ -368,6 +382,8 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *conn_err,
 
 fail:
 	g_dbus_send_message(device->conn, reply);
+	dbus_message_unref(port->msg);
+	port->msg = NULL;
 	g_dbus_remove_watch(device->conn, port->listener_id);
 	port->listener_id = 0;
 }
@@ -431,6 +447,8 @@ failed:
 	g_dbus_remove_watch(device->conn, port->listener_id);
 	port->listener_id = 0;
 	g_dbus_send_message(device->conn, reply);
+	dbus_message_unref(port->msg);
+	port->msg = NULL;
 }
 
 static int connect_port(struct serial_port *port)
-- 
1.7.6


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

* [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure
  2011-08-18 20:10 ` [PATCH 2/5] serial: fix unref of dbus message Gustavo F. Padovan
@ 2011-08-18 20:10   ` Gustavo F. Padovan
  2011-08-18 20:10     ` [PATCH 4/5] serial: Add port->sender Gustavo F. Padovan
  2011-08-19  7:13     ` [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure Luiz Augusto von Dentz
  0 siblings, 2 replies; 11+ messages in thread
From: Gustavo F. Padovan @ 2011-08-18 20:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo F. Padovan

From: "Gustavo F. Padovan" <padovan@profusion.mobi>

Use port->msg has the same effect of using port->io.
---
 serial/port.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/serial/port.c b/serial/port.c
index 3f041a6..bc4b4ea 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -149,7 +149,7 @@ static int port_release(struct serial_port *port)
 	int err = 0;
 
 	if (port->id < 0) {
-		if (port->io) {
+		if (port->msg) {
 			g_io_channel_shutdown(port->io, TRUE, NULL);
 			g_io_channel_unref(port->io);
 			port->io = NULL;
-- 
1.7.6


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

* [PATCH 4/5] serial: Add port->sender
  2011-08-18 20:10   ` [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure Gustavo F. Padovan
@ 2011-08-18 20:10     ` Gustavo F. Padovan
  2011-08-18 20:10       ` [PATCH 5/5] serial: Add support to Disconnect fd passing connections Gustavo F. Padovan
  2011-08-19  7:20       ` [PATCH 4/5] serial: Add port->sender Luiz Augusto von Dentz
  2011-08-19  7:13     ` [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure Luiz Augusto von Dentz
  1 sibling, 2 replies; 11+ messages in thread
From: Gustavo F. Padovan @ 2011-08-18 20:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo F. Padovan

From: "Gustavo F. Padovan" <padovan@profusion.mobi>

Now that port->msg is freed we need a field to track the owner of the
rfcomm connection.
---
 serial/port.c |   13 ++++++++++---
 1 files changed, 10 insertions(+), 3 deletions(-)

diff --git a/serial/port.c b/serial/port.c
index bc4b4ea..784febd 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -85,6 +85,7 @@ struct serial_port {
 	GIOChannel	*io;		/* BtIO channel */
 	guint		listener_id;
 	gboolean	fd_passing;
+	char		*sender;
 	struct serial_device *device;
 };
 
@@ -210,6 +211,7 @@ static void serial_port_free(void *data)
 	}
 
 	g_free(port->uuid);
+	g_free(port->sender);
 	g_free(port);
 }
 
@@ -235,6 +237,7 @@ static void port_owner_exited(DBusConnection *conn, void *user_data)
 	}
 
 	port->listener_id = 0;
+	g_free(port->sender);
 }
 
 static void path_unregister(void *data)
@@ -386,6 +389,8 @@ fail:
 	port->msg = NULL;
 	g_dbus_remove_watch(device->conn, port->listener_id);
 	port->listener_id = 0;
+	g_free(port->sender);
+	port->sender = NULL;
 }
 
 static void get_record_cb(sdp_list_t *recs, int err, gpointer user_data)
@@ -531,6 +536,7 @@ static DBusMessage *_port_connect(DBusConnection *conn, DBusMessage *msg,
 						port_owner_exited, port,
 						NULL);
 	port->msg = dbus_message_ref(msg);
+	port->sender = g_strdup(dbus_message_get_sender(port->msg));
 
 	if (fd)
 		port->fd_passing = TRUE;
@@ -567,7 +573,7 @@ static DBusMessage *port_disconnect(DBusConnection *conn,
 {
 	struct serial_device *device = user_data;
 	struct serial_port *port;
-	const char *dev, *owner, *caller;
+	const char *dev, *caller;
 
 	if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &dev,
 						DBUS_TYPE_INVALID) == FALSE)
@@ -580,15 +586,16 @@ static DBusMessage *port_disconnect(DBusConnection *conn,
 	if (!port->listener_id)
 		return btd_error_not_connected(msg);
 
-	owner = dbus_message_get_sender(port->msg);
 	caller = dbus_message_get_sender(msg);
-	if (!g_str_equal(owner, caller))
+	if (!g_str_equal(port->sender, caller))
 		return btd_error_not_authorized(msg);
 
 	port_release(port);
 
 	g_dbus_remove_watch(conn, port->listener_id);
 	port->listener_id = 0;
+	g_free(port->sender);
+	port->sender = NULL;
 
 	return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
 }
-- 
1.7.6


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

* [PATCH 5/5] serial: Add support to Disconnect fd passing connections
  2011-08-18 20:10     ` [PATCH 4/5] serial: Add port->sender Gustavo F. Padovan
@ 2011-08-18 20:10       ` Gustavo F. Padovan
  2011-08-19  7:34         ` Luiz Augusto von Dentz
  2011-08-19  7:20       ` [PATCH 4/5] serial: Add port->sender Luiz Augusto von Dentz
  1 sibling, 1 reply; 11+ messages in thread
From: Gustavo F. Padovan @ 2011-08-18 20:10 UTC (permalink / raw)
  To: linux-bluetooth; +Cc: Gustavo F. Padovan

From: "Gustavo F. Padovan" <padovan@profusion.mobi>

Disconnect can also be called for connections created with ConnectFD
---
 doc/serial-api.txt |    3 +++
 serial/port.c      |   17 +++++++++++++----
 2 files changed, 16 insertions(+), 4 deletions(-)

diff --git a/doc/serial-api.txt b/doc/serial-api.txt
index 635b3ec..5c9ee9d 100644
--- a/doc/serial-api.txt
+++ b/doc/serial-api.txt
@@ -53,5 +53,8 @@ Methods		fd ConnectFD(string pattern)
 			In that case one of patterns of the Connect method should
 			be suplied instead of the TTY device.
 
+			Connection created with Serial.ConnectFD only accept
+			as parameter the same parameters ConnectFD accepts.
+
 			Possible errors: org.bluez.Error.InvalidArguments
 					 org.bluez.Error.DoesNotExist
diff --git a/serial/port.c b/serial/port.c
index 784febd..e074cef 100644
--- a/serial/port.c
+++ b/serial/port.c
@@ -345,9 +345,6 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *conn_err,
 	bacpy(&req.dst, &device->dst);
 	req.channel = port->channel;
 
-	g_io_channel_unref(port->io);
-	port->io = NULL;
-
 	sk = g_io_channel_unix_get_fd(chan);
 	if (port->fd_passing) {
 		reply = g_dbus_create_reply(port->msg,
@@ -359,6 +356,9 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *conn_err,
 		return;
 	}
 
+	g_io_channel_unref(port->io);
+	port->io = NULL;
+
 	port->id = ioctl(sk, RFCOMMCREATEDEV, &req);
 	if (port->id < 0) {
 		int err = -errno;
@@ -590,7 +590,16 @@ static DBusMessage *port_disconnect(DBusConnection *conn,
 	if (!g_str_equal(port->sender, caller))
 		return btd_error_not_authorized(msg);
 
-	port_release(port);
+	if (port->fd_passing == TRUE) {
+		int sk = g_io_channel_unix_get_fd(port->io);
+		shutdown(sk, SHUT_RDWR);
+
+		g_io_channel_unref(port->io);
+		port->io = NULL;
+	}
+	else {
+		port_release(port);
+	}
 
 	g_dbus_remove_watch(conn, port->listener_id);
 	port->listener_id = 0;
-- 
1.7.6


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

* Re: [PATCH 1/5] serial: add Serial.ConnectFD()
  2011-08-18 20:09 [PATCH 1/5] serial: add Serial.ConnectFD() Gustavo F. Padovan
  2011-08-18 20:10 ` [PATCH 2/5] serial: fix unref of dbus message Gustavo F. Padovan
@ 2011-08-19  7:07 ` Luiz Augusto von Dentz
  2011-08-19 18:49   ` Gustavo Padovan
  1 sibling, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2011-08-19  7:07 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth

Hi Gustavo,

On Thu, Aug 18, 2011 at 11:09 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> From: "Gustavo F. Padovan" <padovan@profusion.mobi>
>
> It's similar to Serial.Connect() but returns the actual RFCOMM file
> descriptor instead of creating a device in /dev
> ---
>  doc/serial-api.txt |   16 ++++++++++++++++
>  serial/port.c      |   36 ++++++++++++++++++++++++++++++++++--
>  2 files changed, 50 insertions(+), 2 deletions(-)
>
> diff --git a/doc/serial-api.txt b/doc/serial-api.txt
> index 5f9bd5f..635b3ec 100644
> --- a/doc/serial-api.txt
> +++ b/doc/serial-api.txt
> @@ -26,6 +26,22 @@ Methods              string Connect(string pattern)
>                                         org.bluez.Error.ConnectionAttemptFailed
>                                         org.bluez.Error.NotSupported
>
> +Methods                fd ConnectFD(string pattern)
> +
> +                       Connects to a specific RFCOMM based service on a
> +                       remote device and returns a file descriptor to talk
> +                        with this device.
> +
> +                       Possible patterns: UUID 128 bit as string
> +                                          Profile short names, e.g: spp, dun
> +                                          RFCOMM channel as string, 1-30
> +
> +                       Possible errors: org.bluez.Error.InvalidArguments
> +                                        org.bluez.Error.InProgress
> +                                        org.bluez.Error.ConnectionAttemptFailed
> +                                        org.bluez.Error.NotSupported
> +
> +

Please mark ConnectFD as experimental, I guess we might want to
complete remove the old Connect with this one when we are allowed to
break the API.

>                void Disconnect(string device)
>
>                        Disconnect a RFCOMM TTY device that has been
> diff --git a/serial/port.c b/serial/port.c
> index d011084..e359716 100644
> --- a/serial/port.c
> +++ b/serial/port.c
> @@ -63,6 +63,10 @@
>  #define MAX_OPEN_TRIES         5
>  #define OPEN_WAIT              300     /* ms. udev node creation retry wait */
>
> +#ifndef DBUS_TYPE_UNIX_FD
> +#define DBUS_TYPE_UNIX_FD -1
> +#endif
> +
>  struct serial_device {
>        DBusConnection  *conn;          /* for name listener handling */
>        bdaddr_t        src;            /* Source (local) address */
> @@ -80,6 +84,7 @@ struct serial_port {
>        int             fd;             /* Opened file descriptor */
>        GIOChannel      *io;            /* BtIO channel */
>        guint           listener_id;
> +       gboolean        fd_passing;
>        struct serial_device *device;
>  };
>
> @@ -329,6 +334,14 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *conn_err,
>        port->io = NULL;
>
>        sk = g_io_channel_unix_get_fd(chan);
> +       if (port->fd_passing) {
> +               reply = g_dbus_create_reply(port->msg,
> +                               DBUS_TYPE_UNIX_FD, &sk,
> +                               DBUS_TYPE_INVALID);
> +               g_dbus_send_message(device->conn, reply);
> +               return;
> +       }
> +

Since you have the message you may just check what is the method call
which triggered the connect, so instead of creating a flag you just do
e.g. if (g_str_equal(dbus_message_get_member(msg), "ConnectFD")).


-- 
Luiz Augusto von Dentz

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

* Re: [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure
  2011-08-18 20:10   ` [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure Gustavo F. Padovan
  2011-08-18 20:10     ` [PATCH 4/5] serial: Add port->sender Gustavo F. Padovan
@ 2011-08-19  7:13     ` Luiz Augusto von Dentz
  2011-08-19 18:56       ` Gustavo Padovan
  1 sibling, 1 reply; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2011-08-19  7:13 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth

Hi Gustavo,

On Thu, Aug 18, 2011 at 11:10 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> From: "Gustavo F. Padovan" <padovan@profusion.mobi>
>
> Use port->msg has the same effect of using port->io.
> ---
>  serial/port.c |    2 +-
>  1 files changed, 1 insertions(+), 1 deletions(-)
>
> diff --git a/serial/port.c b/serial/port.c
> index 3f041a6..bc4b4ea 100644
> --- a/serial/port.c
> +++ b/serial/port.c
> @@ -149,7 +149,7 @@ static int port_release(struct serial_port *port)
>        int err = 0;
>
>        if (port->id < 0) {
> -               if (port->io) {
> +               if (port->msg) {
>                        g_io_channel_shutdown(port->io, TRUE, NULL);
>                        g_io_channel_unref(port->io);
>                        port->io = NULL;

They may have the same effect but I would maintain the current one
since we are only using port->io and changing it to port->msg may be
unclear when reading the code without knowing the behavior.

-- 
Luiz Augusto von Dentz

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

* Re: [PATCH 4/5] serial: Add port->sender
  2011-08-18 20:10     ` [PATCH 4/5] serial: Add port->sender Gustavo F. Padovan
  2011-08-18 20:10       ` [PATCH 5/5] serial: Add support to Disconnect fd passing connections Gustavo F. Padovan
@ 2011-08-19  7:20       ` Luiz Augusto von Dentz
  1 sibling, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2011-08-19  7:20 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth

Hi Gustavo,

On Thu, Aug 18, 2011 at 11:10 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> From: "Gustavo F. Padovan" <padovan@profusion.mobi>
>
> Now that port->msg is freed we need a field to track the owner of the
> rfcomm connection.
> ---
>  serial/port.c |   13 ++++++++++---
>  1 files changed, 10 insertions(+), 3 deletions(-)
>
> diff --git a/serial/port.c b/serial/port.c
> index bc4b4ea..784febd 100644
> --- a/serial/port.c
> +++ b/serial/port.c
> @@ -85,6 +85,7 @@ struct serial_port {
>        GIOChannel      *io;            /* BtIO channel */
>        guint           listener_id;
>        gboolean        fd_passing;
> +       char            *sender;
>        struct serial_device *device;
>  };
>
> @@ -210,6 +211,7 @@ static void serial_port_free(void *data)
>        }
>
>        g_free(port->uuid);
> +       g_free(port->sender);
>        g_free(port);
>  }
>
> @@ -235,6 +237,7 @@ static void port_owner_exited(DBusConnection *conn, void *user_data)
>        }
>
>        port->listener_id = 0;
> +       g_free(port->sender);
>  }
>
>  static void path_unregister(void *data)
> @@ -386,6 +389,8 @@ fail:
>        port->msg = NULL;
>        g_dbus_remove_watch(device->conn, port->listener_id);
>        port->listener_id = 0;
> +       g_free(port->sender);
> +       port->sender = NULL;
>  }
>
>  static void get_record_cb(sdp_list_t *recs, int err, gpointer user_data)
> @@ -531,6 +536,7 @@ static DBusMessage *_port_connect(DBusConnection *conn, DBusMessage *msg,
>                                                port_owner_exited, port,
>                                                NULL);
>        port->msg = dbus_message_ref(msg);
> +       port->sender = g_strdup(dbus_message_get_sender(port->msg));
>
>        if (fd)
>                port->fd_passing = TRUE;
> @@ -567,7 +573,7 @@ static DBusMessage *port_disconnect(DBusConnection *conn,
>  {
>        struct serial_device *device = user_data;
>        struct serial_port *port;
> -       const char *dev, *owner, *caller;
> +       const char *dev, *caller;
>
>        if (dbus_message_get_args(msg, NULL, DBUS_TYPE_STRING, &dev,
>                                                DBUS_TYPE_INVALID) == FALSE)
> @@ -580,15 +586,16 @@ static DBusMessage *port_disconnect(DBusConnection *conn,
>        if (!port->listener_id)
>                return btd_error_not_connected(msg);
>
> -       owner = dbus_message_get_sender(port->msg);
>        caller = dbus_message_get_sender(msg);
> -       if (!g_str_equal(owner, caller))
> +       if (!g_str_equal(port->sender, caller))
>                return btd_error_not_authorized(msg);
>
>        port_release(port);
>
>        g_dbus_remove_watch(conn, port->listener_id);
>        port->listener_id = 0;
> +       g_free(port->sender);
> +       port->sender = NULL;
>
>        return g_dbus_create_reply(msg, DBUS_TYPE_INVALID);
>  }
> --
> 1.7.6

So port->msg wasn't leaking, well it was when we do port_free, we use
it for checking the sender, what you probably want to do is to remove
the need of port->msg by just storing its sender, in this case I would
merge the patches so we don't break the code while doing the change.

-- 
Luiz Augusto von Dentz

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

* Re: [PATCH 5/5] serial: Add support to Disconnect fd passing connections
  2011-08-18 20:10       ` [PATCH 5/5] serial: Add support to Disconnect fd passing connections Gustavo F. Padovan
@ 2011-08-19  7:34         ` Luiz Augusto von Dentz
  0 siblings, 0 replies; 11+ messages in thread
From: Luiz Augusto von Dentz @ 2011-08-19  7:34 UTC (permalink / raw)
  To: Gustavo F. Padovan; +Cc: linux-bluetooth

Hi Gustavo,

On Thu, Aug 18, 2011 at 11:10 PM, Gustavo F. Padovan
<padovan@profusion.mobi> wrote:
> -       port_release(port);
> +       if (port->fd_passing == TRUE) {
> +               int sk = g_io_channel_unix_get_fd(port->io);
> +               shutdown(sk, SHUT_RDWR);
> +
> +               g_io_channel_unref(port->io);
> +               port->io = NULL;
> +       }

How about moving this code inside port_release? It is probably
necessary to always call shutdown if we really want to make sure the
port is release, so even if the sender disconnects from bus it doesn't
mean it has released its reference, the fd may be passed again
(unlikely but possible) to another process.

-- 
Luiz Augusto von Dentz

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

* Re: [PATCH 1/5] serial: add Serial.ConnectFD()
  2011-08-19  7:07 ` [PATCH 1/5] serial: add Serial.ConnectFD() Luiz Augusto von Dentz
@ 2011-08-19 18:49   ` Gustavo Padovan
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Padovan @ 2011-08-19 18:49 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-19 10:07:30 +0300]:

> Hi Gustavo,
> 
> On Thu, Aug 18, 2011 at 11:09 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> > From: "Gustavo F. Padovan" <padovan@profusion.mobi>
> >
> > It's similar to Serial.Connect() but returns the actual RFCOMM file
> > descriptor instead of creating a device in /dev
> > ---
> >  doc/serial-api.txt |   16 ++++++++++++++++
> >  serial/port.c      |   36 ++++++++++++++++++++++++++++++++++--
> >  2 files changed, 50 insertions(+), 2 deletions(-)
> >
> > diff --git a/doc/serial-api.txt b/doc/serial-api.txt
> > index 5f9bd5f..635b3ec 100644
> > --- a/doc/serial-api.txt
> > +++ b/doc/serial-api.txt
> > @@ -26,6 +26,22 @@ Methods              string Connect(string pattern)
> >                                         org.bluez.Error.ConnectionAttemptFailed
> >                                         org.bluez.Error.NotSupported
> >
> > +Methods                fd ConnectFD(string pattern)
> > +
> > +                       Connects to a specific RFCOMM based service on a
> > +                       remote device and returns a file descriptor to talk
> > +                        with this device.
> > +
> > +                       Possible patterns: UUID 128 bit as string
> > +                                          Profile short names, e.g: spp, dun
> > +                                          RFCOMM channel as string, 1-30
> > +
> > +                       Possible errors: org.bluez.Error.InvalidArguments
> > +                                        org.bluez.Error.InProgress
> > +                                        org.bluez.Error.ConnectionAttemptFailed
> > +                                        org.bluez.Error.NotSupported
> > +
> > +
> 
> Please mark ConnectFD as experimental, I guess we might want to
> complete remove the old Connect with this one when we are allowed to
> break the API.
> 
> >                void Disconnect(string device)
> >
> >                        Disconnect a RFCOMM TTY device that has been
> > diff --git a/serial/port.c b/serial/port.c
> > index d011084..e359716 100644
> > --- a/serial/port.c
> > +++ b/serial/port.c
> > @@ -63,6 +63,10 @@
> >  #define MAX_OPEN_TRIES         5
> >  #define OPEN_WAIT              300     /* ms. udev node creation retry wait */
> >
> > +#ifndef DBUS_TYPE_UNIX_FD
> > +#define DBUS_TYPE_UNIX_FD -1
> > +#endif
> > +
> >  struct serial_device {
> >        DBusConnection  *conn;          /* for name listener handling */
> >        bdaddr_t        src;            /* Source (local) address */
> > @@ -80,6 +84,7 @@ struct serial_port {
> >        int             fd;             /* Opened file descriptor */
> >        GIOChannel      *io;            /* BtIO channel */
> >        guint           listener_id;
> > +       gboolean        fd_passing;
> >        struct serial_device *device;
> >  };
> >
> > @@ -329,6 +334,14 @@ static void rfcomm_connect_cb(GIOChannel *chan, GError *conn_err,
> >        port->io = NULL;
> >
> >        sk = g_io_channel_unix_get_fd(chan);
> > +       if (port->fd_passing) {
> > +               reply = g_dbus_create_reply(port->msg,
> > +                               DBUS_TYPE_UNIX_FD, &sk,
> > +                               DBUS_TYPE_INVALID);
> > +               g_dbus_send_message(device->conn, reply);
> > +               return;
> > +       }
> > +
> 
> Since you have the message you may just check what is the method call
> which triggered the connect, so instead of creating a flag you just do
> e.g. if (g_str_equal(dbus_message_get_member(msg), "ConnectFD")).

I want to use fd_passing flag in the Disconnect part. We can't rely on the DBus
msg for this, once it freed (one of the following patches).

	Gustavo

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

* Re: [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure
  2011-08-19  7:13     ` [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure Luiz Augusto von Dentz
@ 2011-08-19 18:56       ` Gustavo Padovan
  0 siblings, 0 replies; 11+ messages in thread
From: Gustavo Padovan @ 2011-08-19 18:56 UTC (permalink / raw)
  To: Luiz Augusto von Dentz; +Cc: linux-bluetooth

Hi Luiz,

* Luiz Augusto von Dentz <luiz.dentz@gmail.com> [2011-08-19 10:13:16 +0300]:

> Hi Gustavo,
> 
> On Thu, Aug 18, 2011 at 11:10 PM, Gustavo F. Padovan
> <padovan@profusion.mobi> wrote:
> > From: "Gustavo F. Padovan" <padovan@profusion.mobi>
> >
> > Use port->msg has the same effect of using port->io.
> > ---
> >  serial/port.c |    2 +-
> >  1 files changed, 1 insertions(+), 1 deletions(-)
> >
> > diff --git a/serial/port.c b/serial/port.c
> > index 3f041a6..bc4b4ea 100644
> > --- a/serial/port.c
> > +++ b/serial/port.c
> > @@ -149,7 +149,7 @@ static int port_release(struct serial_port *port)
> >        int err = 0;
> >
> >        if (port->id < 0) {
> > -               if (port->io) {
> > +               if (port->msg) {
> >                        g_io_channel_shutdown(port->io, TRUE, NULL);
> >                        g_io_channel_unref(port->io);
> >                        port->io = NULL;
> 
> They may have the same effect but I would maintain the current one
> since we are only using port->io and changing it to port->msg may be
> unclear when reading the code without knowing the behavior.

Yes, but I want to change the behavior of port->io since I don't to create
another struct member to mantain the IO channel.
Maybe we can keep it here, I'll check the code.

	Gustavo

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

end of thread, other threads:[~2011-08-19 18:56 UTC | newest]

Thread overview: 11+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2011-08-18 20:09 [PATCH 1/5] serial: add Serial.ConnectFD() Gustavo F. Padovan
2011-08-18 20:10 ` [PATCH 2/5] serial: fix unref of dbus message Gustavo F. Padovan
2011-08-18 20:10   ` [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure Gustavo F. Padovan
2011-08-18 20:10     ` [PATCH 4/5] serial: Add port->sender Gustavo F. Padovan
2011-08-18 20:10       ` [PATCH 5/5] serial: Add support to Disconnect fd passing connections Gustavo F. Padovan
2011-08-19  7:34         ` Luiz Augusto von Dentz
2011-08-19  7:20       ` [PATCH 4/5] serial: Add port->sender Luiz Augusto von Dentz
2011-08-19  7:13     ` [PATCH 3/5] serial: use port->msg to track rfcomm connection procedure Luiz Augusto von Dentz
2011-08-19 18:56       ` Gustavo Padovan
2011-08-19  7:07 ` [PATCH 1/5] serial: add Serial.ConnectFD() Luiz Augusto von Dentz
2011-08-19 18:49   ` Gustavo Padovan

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.