Re: [Bluez-devel] HCID D-Bus (Sig Fault)

[Claudio Takahasi <cktakahasi@gmail.com>]

[-- Attachment #1.1: Type: text/plain, Size: 3034 bytes --]

Hi Marcel,

I am sending a new patch that contains the following enhancements:
1. Support multiple D-Bus versions(0.23, 0.30...) 
2. Fixed error handling ServiceDoesNotExist, NoReply, UnknownService, ...
This condition is causing sig fault due the wrong pin value. Scenario: D-Bus 
pin helper enabled 
and an error different than wrong argument triggered.


Regards,
Claudio.

On 9/9/05, Marcel Holtmann <marcel@holtmann.org> wrote:
> 
> Hi Sjoerd,
> 
> > > having 0.33 is better than 0.23 and this is what Debian unstable still
> > > uses. However lets make a list of the distributions:
> > >
> > > SuSE Linux 10.0 D-Bus 0.35
> > > Fedora Core 4 D-Bus 0.33
> > > Ubuntu Breezy D-Bus 0.36
> > > Debian unstable D-Bus 0.23
> > Debian experimental D-Bus 0.50
> 
> this is good.
> 
> > > The only bad guy is Debian unstable and I don't care anymore. They use
> > > Xorg finally and have GCC 4.0 etc., but they can't move over to the 
> next
> > > D-Bus generation. I think that is their problem now and the release of
> > > bluez-utils-0.22 will definitely drop the support for the old D-Bus 
> 0.23
> > > version.
> >
> > D-bus >= 0.3x has been in debian experimental for quite some time. There 
> are
> > various transitions going on in debian unstable currently which makes it
> > inappropriate to do a dbus transition now too.
> 
> Last time I looked at it, the complete KDE part was broken and full of
> wrong dependencies. So breaking D-Bus wouldn't even be notified ;)
> 
> > > I like to base everything around the current D-Bus 0.50 release, 
> because
> > > the C API should be quite stable now. I will accept small workarounds 
> to
> > > make D-Bus 0.3x work, but as soon as the distributions update their
> > > D-Bus libraries I will drop them.
> > >
> > > This is not a nice thing to do, I know, but a wrong usage of D-Bus 
> lets
> > > hcid segfault and this is not what I want.
> >
> > As one of debian's dbus maintainers i would advise to indeed base 
> everything on
> > dbus >= 0.50. Hopefully it won't take too long before we can move the 
> new dbus
> > from experimental to unstable, so the workarounds for old dbus release 
> are just
> > not worth it.
> 
> This means I will release bluez-utils-2.21 on the weekend, because of
> some memory leak fixes and then I will drop D-Bus 0.23 support. So if
> any patches need to go in before, please send them now.
> 
> Regards
> 
> Marcel
> 
> 
> 
> 
> -------------------------------------------------------
> SF.Net email is Sponsored by the Better Software Conference & EXPO
> September 19-22, 2005 * San Francisco, CA * Development Lifecycle 
> Practices
> Agile & Plan-Driven Development * Managing Projects & Teams * Testing & QA
> Security * Process Improvement & Measurement * http://www.sqe.com/bsce5sf
> _______________________________________________
> Bluez-devel mailing list
> Bluez-devel@lists.sourceforge.net
> https://lists.sourceforge.net/lists/listinfo/bluez-devel
>

[-- Attachment #1.2: Type: text/html, Size: 4031 bytes --]

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: hcid_dbus_0002.patch --]
[-- Type: text/x-patch; name="hcid_dbus_0002.patch", Size: 13505 bytes --]

--- bluez-utils-2.21.orig/hcid/dbus-compat.h	1969-12-31 21:00:00.000000000 -0300
+++ bluez-hcid-0.0.3/hcid/dbus-compat.h	2005-09-12 16:42:22.382630376 -0300
@@ -0,0 +1,114 @@
+/*
+ *
+ *  BlueZ - Bluetooth protocol stack for Linux
+ *
+ *  Copyright (C) 2000-2001  Qualcomm Incorporated
+ *  Copyright (C) 2002-2003  Maxim Krasnyansky <maxk@qualcomm.com>
+ *  Copyright (C) 2002-2005  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 version 2 as
+ *  published by the Free Software Foundation;
+ *
+ *  THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS
+ *  OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY,
+ *  FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT OF THIRD PARTY RIGHTS.
+ *  IN NO EVENT SHALL THE COPYRIGHT HOLDER(S) AND AUTHOR(S) BE LIABLE FOR ANY
+ *  CLAIM, OR ANY SPECIAL INDIRECT OR CONSEQUENTIAL DAMAGES, OR ANY DAMAGES 
+ *  WHATSOEVER RESULTING FROM LOSS OF USE, DATA OR PROFITS, WHETHER IN AN 
+ *  ACTION OF CONTRACT, NEGLIGENCE OR OTHER TORTIOUS ACTION, ARISING OUT OF 
+ *  OR IN CONNECTION WITH THE USE OR PERFORMANCE OF THIS SOFTWARE.
+ *
+ *  ALL LIABILITY, INCLUDING LIABILITY FOR INFRINGEMENT OF ANY PATENTS, 
+ *  COPYRIGHTS, TRADEMARKS OR OTHER RIGHTS, RELATING TO USE OF THIS 
+ *  SOFTWARE IS DISCLAIMED.
+ *
+ *
+ *  $Id: dbus-compat.h,v 1.24 2005/09/08 18:32:41 holtmann Exp $
+ */
+
+#ifndef __H_DBUS_COMPAT_H__
+#define __H_DBUS_COMPAT_H__
+
+#ifdef HAVE_CONFIG_H
+#include <config.h>
+#endif
+
+#define CHECK_DBUS_VERSION(major, minor) \
+		(DBUS_MAJOR_VER > (major) || \
+		 	(DBUS_MAJOR_VER == (major) && DBUS_MINOR_VER >= (minor)))
+	
+#if CHECK_DBUS_VERSION(0, 30)
+	/* For D-Bus version >= 0.30 */
+	/****** pending, requires chnages in the makefile  *****/
+#else /* < 0.30 */
+	/* For D-Bus version older than 0.30 */
+	
+#endif
+
+#if  HAVE_DBUS_MESSAGE_ITER_GET_BASIC
+	#warning "D-Bus > 0.30"
+	
+	/* Get iterators */
+	#define DBUS_MSG_ITER_GET_STR(iter, retvar) \
+			dbus_message_iter_get_basic((iter), &(retvar))
+	#define DBUS_MSG_ITER_GET_BYTE(iter, retvar) \
+			dbus_message_iter_get_basic((iter), &(retvar))
+			
+	/* Append iterators */
+	#define DBUS_MSG_APPEND_ITER_INIT(a, b) \
+			dbus_message_iter_init_append(a, b)	
+	#define DBUS_MSG_ITER_APPEND_BYTE(iter, val) \
+			dbus_message_iter_append_basic((iter), DBUS_TYPE_BYTE, &(val))
+	#define DBUS_MSG_ITER_APPEND_STR(iter, val) \
+			dbus_message_iter_append_basic((iter), DBUS_TYPE_STRING, &(val))
+	#define DBUS_MSG_ITER_APPEND_BOOL(iter, val) \
+			dbus_message_iter_append_basic((iter), DBUS_TYPE_BOOLEAN, &(val))
+	#define DBUS_MSG_ITER_APPEND_INT32(iter, val) \
+			dbus_message_iter_append_basic((iter), DBUS_TYPE_INT32, &(val))
+	#define DBUS_MSG_ITER_APPEND_UINT32(iter, val) \
+			dbus_message_iter_append_basic((iter), DBUS_TYPE_UINT32, &(val))
+
+	inline void DBUS_MSG_ITER_APPEND_BYTE_ARRAY(DBusMessageIter *iter, uint8_t *ptr, uint8_t size) 
+	{
+		DBusMessageIter array_iter;
+		const char array_sig[] = DBUS_TYPE_BYTE_AS_STRING;
+		int i;
+		
+		dbus_message_iter_open_container(iter, DBUS_TYPE_ARRAY, array_sig, &array_iter);
+		for ( i = 0; i < size; i++ ) {
+			DBUS_MSG_ITER_APPEND_BYTE(&array_iter, ptr);
+		}
+		dbus_message_iter_close_container(iter, &array_iter);
+	}
+#else
+	#warning "D-Bus < 0.30"
+	
+	/* Get iterators */
+	#define DBUS_MSG_ITER_GET_STR(iter, retvar) \
+			retvar = dbus_message_iter_get_string(iter)
+	#define DBUS_MSG_ITER_GET_BYTE(iter, retvar) \
+			retvar = dbus_message_iter_get_byte(iter)
+			
+	/* Append iterators */
+	#define DBUS_MSG_APPEND_ITER_INIT(a, b) \
+			dbus_message_append_iter_init(a, b)
+	#define DBUS_MSG_ITER_APPEND_BYTE(iter, val) \
+			dbus_message_iter_append_byte((iter), (val))
+	#define DBUS_MSG_ITER_APPEND_STR(iter, val) \
+			dbus_message_iter_append_string((iter), (val))
+	#define DBUS_MSG_ITER_APPEND_BOOL(iter, val) \
+			dbus_message_iter_append_boolean((iter), (val))
+	#define DBUS_MSG_ITER_APPEND_INT32(iter, val) \
+			dbus_message_iter_append_int32((iter), (val))
+	#define DBUS_MSG_ITER_APPEND_UINT32(iter, val) \
+			dbus_message_iter_append_uint32((iter), (val))
+	#define DBUS_MSG_ITER_APPEND_BYTE_ARRAY(iter, val, size) \
+		dbus_message_iter_append_byte_array(iter, val, size)
+
+#endif
+
+
+
+#endif /*__H_DBUS_COMPAT_H__*/
--- bluez-utils-2.21.orig/hcid/dbus.c	2005-08-27 10:37:14.000000000 -0300
+++ bluez-hcid-0.0.3/hcid/dbus.c	2005-09-12 17:19:44.362797584 -0300
@@ -45,15 +45,20 @@
 #include "glib-ectomy.h"
 
 #include "hcid.h"
+#include "dbus-compat.h"
 
 static DBusConnection *connection;
 
 #define TIMEOUT (30 * 1000)		/* 30 seconds */
 
-#define SERVICE_NAME "org.bluez.PinAgent"
-#define INTERFACE_NAME SERVICE_NAME
-#define REQUEST_NAME "PinRequest"
-#define PATH_NAME "/org/bluez/PinAgent"
+#define PINAGENT_DBUS_SERVICE		"org.bluez.PinAgent"
+#define PINAGENT_DBUS_INTERFACE 	PINAGENT_DBUS_SERVICE
+#define PINAGENT_DBUS_SVC_NAME_REQ 		"PinRequest"
+#define PINAGENT_DBUS_PATH 			"/org/bluez/PinAgent"
+
+#define HCI_DBUS_SERVICE		"org.bluez.bluetoothd.hci"
+#define HCI_DBUS_INTERFACE		HCI_DBUS_SERVICE
+#define HCI_DBUS_PATH			"/org/bluez/bluetoothd/hci"
 
 #define WRONG_ARGS_ERROR "org.bluez.Error.WrongArgs"
 
@@ -68,26 +73,34 @@
 	pin_code_reply_cp pr;
 	DBusMessage *message;
 	DBusMessageIter iter;
-	int type;
+	int arg_type;
+	int msg_type;
 	size_t len;
 	char *pin;
+	const char *error_msg;
 
 	message = dbus_pending_call_steal_reply(call);
+	msg_type = dbus_message_get_type(message);
+	dbus_message_iter_init(message, &iter);
 
-	if (dbus_message_is_error(message, WRONG_ARGS_ERROR))
-		goto error;
+	if(msg_type == DBUS_MESSAGE_TYPE_ERROR) {
+		DBUS_MSG_ITER_GET_STR(&iter, error_msg);
+		/* handling WRONG_ARGS_ERROR, DBUS_ERROR_NO_REPLY, DBUS_ERROR_SERVICE_UNKNOWN */
+		syslog(LOG_ERR, "%s: %s", dbus_message_get_error_name(message), error_msg);
 
-	dbus_message_iter_init(message, &iter);
+		hci_send_cmd(req->dev, OGF_LINK_CTL,
+			     OCF_PIN_CODE_NEG_REPLY, 6, &req->bda);
+	} else {
+		/* check signature */
+		arg_type = dbus_message_iter_get_arg_type(&iter);	
+		if (arg_type != DBUS_TYPE_STRING) {
+			syslog(LOG_ERR, "Wrong reply signature: expected PIN");
 
-	type = dbus_message_iter_get_arg_type(&iter);
-	if (type != DBUS_TYPE_STRING)
-		goto error;
-
-#ifdef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
-	dbus_message_iter_get_basic(&iter, &pin);
-#else
-	pin = dbus_message_iter_get_string(&iter);
-#endif
+			hci_send_cmd(req->dev, OGF_LINK_CTL,
+				     OCF_PIN_CODE_NEG_REPLY, 6, &req->bda);
+		} else {
+		
+			DBUS_MSG_ITER_GET_STR(&iter, pin);
 
 	len = strlen(pin);
 
@@ -97,15 +110,12 @@
 	pr.pin_len = len;
 	hci_send_cmd(req->dev, OGF_LINK_CTL, OCF_PIN_CODE_REPLY,
 						PIN_CODE_REPLY_CP_SIZE, &pr);
+		}
+
+	}
 
 	dbus_message_unref(message);
 	dbus_pending_call_unref(call);
-
-	return;
-
-error:
-	hci_send_cmd(req->dev, OGF_LINK_CTL,
-				OCF_PIN_CODE_NEG_REPLY, 6, &req->bda);
 }
 
 static void free_pin_req(void *req)
@@ -116,16 +126,13 @@
 void hcid_dbus_request_pin(int dev, struct hci_conn_info *ci)
 {
 	DBusMessage *message;
-#ifdef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
 	uint8_t *addr = (uint8_t *) &ci->bdaddr;
-#else
 	DBusMessageIter iter;
-#endif
 	DBusPendingCall *pending = NULL;
 	struct pin_request *req;
 
-	message = dbus_message_new_method_call(SERVICE_NAME, PATH_NAME,
-						INTERFACE_NAME, REQUEST_NAME);
+	message = dbus_message_new_method_call(PINAGENT_DBUS_SERVICE, PINAGENT_DBUS_PATH,
+						PINAGENT_DBUS_INTERFACE, PINAGENT_DBUS_SVC_NAME_REQ);
 	if (message == NULL) {
 		syslog(LOG_ERR, "Couldn't allocate D-BUS message");
 		goto failed;
@@ -135,17 +142,9 @@
 	req->dev = dev;
 	bacpy(&req->bda, &ci->bdaddr);
 
-#ifdef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
-	dbus_message_append_args(message, DBUS_TYPE_BOOLEAN, &ci->out,
-			DBUS_TYPE_ARRAY, DBUS_TYPE_BYTE,
-			&addr, sizeof(bdaddr_t), DBUS_TYPE_INVALID);
-#else
-	dbus_message_append_iter_init(message, &iter);
-
-	dbus_message_iter_append_boolean(&iter, ci->out);
-	dbus_message_iter_append_byte_array(&iter,
-			(unsigned char *) &ci->bdaddr, sizeof(ci->bdaddr));
-#endif
+	DBUS_MSG_APPEND_ITER_INIT(message, &iter);
+	DBUS_MSG_ITER_APPEND_BOOL(&iter, ci->out);
+	DBUS_MSG_ITER_APPEND_BYTE_ARRAY(&iter, addr, sizeof(bdaddr_t));
 
 	if (dbus_connection_send_with_reply(connection, message,
 					&pending, TIMEOUT) == FALSE) {
@@ -171,30 +170,21 @@
 void hcid_dbus_inquiry_start(bdaddr_t *local)
 {
 	DBusMessage *message;
-#ifndef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
 	DBusMessageIter iter;
-#endif
 	char *local_addr;	
 	bdaddr_t tmp;
 
 	baswap(&tmp, local); local_addr = batostr(&tmp);
 
-	message = dbus_message_new_signal("/org/bluez/DevAgent",
-				"org.bluez.DevAgent", "InquiryStart");
+	message = dbus_message_new_signal(HCI_DBUS_PATH,
+				HCI_DBUS_INTERFACE, "InquiryStart");
 	if (message == NULL) {
 		syslog(LOG_ERR, "Can't allocate D-BUS inquiry start message");
 		goto failed;
 	}
 
-#ifdef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
-	dbus_message_append_args(message,
-					DBUS_TYPE_STRING, &local_addr,
-					DBUS_TYPE_INVALID);
-#else
-	dbus_message_append_iter_init(message, &iter);
-
-	dbus_message_iter_append_string(&iter, local_addr);
-#endif
+	DBUS_MSG_APPEND_ITER_INIT(message, &iter);
+	DBUS_MSG_ITER_APPEND_STR(&iter, local_addr);
 
 	if (dbus_connection_send(connection, message, NULL) == FALSE) {
 		syslog(LOG_ERR, "Can't send D-BUS inquiry start message");
@@ -214,30 +204,21 @@
 void hcid_dbus_inquiry_complete(bdaddr_t *local)
 {
 	DBusMessage *message;
-#ifndef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
 	DBusMessageIter iter;
-#endif
 	char *local_addr;
 	bdaddr_t tmp;
 
 	baswap(&tmp, local); local_addr = batostr(&tmp);
 
-	message = dbus_message_new_signal("/org/bluez/DevAgent",
-				"org.bluez.DevAgent", "InquiryComplete");
+	message = dbus_message_new_signal(HCI_DBUS_PATH,
+				HCI_DBUS_INTERFACE, "InquiryComplete");
 	if (message == NULL) {
 		syslog(LOG_ERR, "Can't allocate D-BUS inquiry complete message");
 		goto failed;
 	}
 
-#ifdef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
-	dbus_message_append_args(message,
-					DBUS_TYPE_STRING, &local_addr,
-					DBUS_TYPE_INVALID);
-#else
-	dbus_message_append_iter_init(message, &iter);
-
-	dbus_message_iter_append_string(&iter, local_addr);
-#endif
+	DBUS_MSG_APPEND_ITER_INIT(message, &iter);
+	DBUS_MSG_ITER_APPEND_STR(&iter, local_addr);
 
 	if (dbus_connection_send(connection, message, NULL) == FALSE) {
 		syslog(LOG_ERR, "Can't send D-BUS inquiry complete message");
@@ -257,37 +238,25 @@
 void hcid_dbus_inquiry_result(bdaddr_t *local, bdaddr_t *peer, uint32_t class, int8_t rssi)
 {
 	DBusMessage *message;
-#ifndef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
 	DBusMessageIter iter;
-#endif
 	char *local_addr, *peer_addr;
 	bdaddr_t tmp;
 
 	baswap(&tmp, local); local_addr = batostr(&tmp);
 	baswap(&tmp, peer); peer_addr = batostr(&tmp);
 
-	message = dbus_message_new_signal("/org/bluez/DevAgent",
-				"org.bluez.DevAgent", "InquiryResult");
+	message = dbus_message_new_signal(HCI_DBUS_PATH,
+				HCI_DBUS_INTERFACE, "InquiryResult");
 	if (message == NULL) {
 		syslog(LOG_ERR, "Can't allocate D-BUS inquiry result message");
 		goto failed;
 	}
 
-#ifdef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
-	dbus_message_append_args(message,
-					DBUS_TYPE_STRING, &local_addr,
-					DBUS_TYPE_STRING, &peer_addr,
-					DBUS_TYPE_UINT32, &class,
-					DBUS_TYPE_INT32, &rssi,
-					DBUS_TYPE_INVALID);
-#else
-	dbus_message_append_iter_init(message, &iter);
-
-	dbus_message_iter_append_string(&iter, local_addr);
-	dbus_message_iter_append_string(&iter, peer_addr);
-	dbus_message_iter_append_uint32(&iter, class);
-	dbus_message_iter_append_int32(&iter, rssi);
-#endif
+	DBUS_MSG_APPEND_ITER_INIT(message, &iter);
+	DBUS_MSG_ITER_APPEND_STR(&iter, local_addr);
+	DBUS_MSG_ITER_APPEND_STR(&iter, peer_addr);
+	DBUS_MSG_ITER_APPEND_UINT32(&iter, class);
+	DBUS_MSG_ITER_APPEND_INT32(&iter, rssi);
 
 	if (dbus_connection_send(connection, message, NULL) == FALSE) {
 		syslog(LOG_ERR, "Can't send D-BUS inquiry result message");
@@ -308,35 +277,24 @@
 void hcid_dbus_remote_name(bdaddr_t *local, bdaddr_t *peer, char *name)
 {
 	DBusMessage *message;
-#ifndef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
 	DBusMessageIter iter;
-#endif
 	char *local_addr, *peer_addr;
 	bdaddr_t tmp;
 
 	baswap(&tmp, local); local_addr = batostr(&tmp);
 	baswap(&tmp, peer); peer_addr = batostr(&tmp);
 
-	message = dbus_message_new_signal("/org/bluez/DevAgent",
-				"org.bluez.DevAgent", "RemoteName");
+	message = dbus_message_new_signal(HCI_DBUS_PATH,
+				HCI_DBUS_INTERFACE, "RemoteName");
 	if (message == NULL) {
 		syslog(LOG_ERR, "Can't allocate D-BUS remote name message");
 		goto failed;
 	}
 
-#ifdef HAVE_DBUS_MESSAGE_ITER_GET_BASIC
-	dbus_message_append_args(message,
-					DBUS_TYPE_STRING, &local_addr,
-					DBUS_TYPE_STRING, &peer_addr,
-					DBUS_TYPE_STRING, &name,
-					DBUS_TYPE_INVALID);
-#else
-	dbus_message_append_iter_init(message, &iter);
-
-	dbus_message_iter_append_string(&iter, local_addr);
-	dbus_message_iter_append_string(&iter, peer_addr);
-	dbus_message_iter_append_string(&iter, name);
-#endif
+	DBUS_MSG_APPEND_ITER_INIT(message, &iter);
+	DBUS_MSG_ITER_APPEND_STR(&iter, local_addr);
+	DBUS_MSG_ITER_APPEND_STR(&iter, peer_addr);
+	DBUS_MSG_ITER_APPEND_STR(&iter, name);
 
 	if (dbus_connection_send(connection, message, NULL) == FALSE) {
 		syslog(LOG_ERR, "Can't send D-BUS remote name message");