[patch] fix the bug of obex_object_resume(), obex_server() in openobex

[patch] fix the bug of obex_object_resume(), obex_server() in openobex

[Zhao Forrest]

Hi,

When developing the PBAP server in obexd we found that
obex_object_resume() can't work as expected. After debugging openobex
we post this patch to fix the bug. We did the internal test with PBAP
server/client. The below patch fixes the bug in obex_object_resume()
and obex_server().

Thanks,
Forrest

diff --git a/lib/obex_object.c b/lib/obex_object.c
index 482e6a7..0af0108 100644
--- a/lib/obex_object.c
+++ b/lib/obex_object.c
@@ -908,24 +908,38 @@ int obex_object_suspend(obex_object_t *object)

 int obex_object_resume(obex_t *self, obex_object_t *object)
 {
+	int ret;
+
 	if (!object->suspend)
 		return 0;

 	object->suspend = 0;

-	if (object->first_packet_sent && !object->continue_received)
+ 	if (object->first_packet_sent && !object->continue_received)
 		return 0;

-	if (obex_object_send(self, object, TRUE, FALSE) < 0) {
-		obex_deliver_event(self, OBEX_EV_LINKERR, object->opcode, 0, TRUE);
+	ret = obex_object_send(self, object, TRUE, FALSE);
+
+	if (ret < 0) {
+		obex_deliver_event(self, OBEX_EV_LINKERR, object->opcode &
~OBEX_FINAL, 0, TRUE);
 		return -1;
-	}
+	} else if (ret == 0){

-	obex_deliver_event(self, OBEX_EV_PROGRESS, object->opcode, 0, FALSE);
+		obex_deliver_event(self, OBEX_EV_PROGRESS, object->opcode &
~OBEX_FINAL, 0, FALSE);
+		object->first_packet_sent = 1;
+		object->continue_received = 0;
+	} else {
+		if (self->state & MODE_SRV) {
+			obex_deliver_event(self, OBEX_EV_REQDONE, object->opcode &
~OBEX_FINAL, 0, TRUE);
+			self->state = MODE_SRV | STATE_IDLE;
+			return 0;
+		}
+	}

-	self->state = MODE_CLI | STATE_REC;
-	object->first_packet_sent = 1;
-	object->continue_received = 0;
+	if (self->state & MODE_SRV)
+		self->state = MODE_SRV | STATE_REC;
+	else
+		self->state = MODE_CLI | STATE_REC;

 	return 0;
 }
diff --git a/lib/obex_server.c b/lib/obex_server.c
index f27c8ee..cf19529 100644
--- a/lib/obex_server.c
+++ b/lib/obex_server.c
@@ -159,7 +159,7 @@ int obex_server(obex_t *self, buf_t *msg, int final)
 			} else
 				obex_deliver_event(self, OBEX_EV_PROGRESS, cmd, 0, FALSE);
 			break; /* Stay in this state if not final */
-		} else {
+		} else if (!self->object->first_packet_sent) {
 			DEBUG(4, "We got a request!\n");
 			/* More connect-magic woodoo stuff */
 			if (cmd == OBEX_CMD_CONNECT)
@@ -234,10 +234,17 @@ int obex_server(obex_t *self, buf_t *msg, int final)
 		 * See Obex spec v1.2, chapter 3.2, page 21 and 22.
 		 * See also example on chapter 7.3, page 47.
 		 * So, force the final bit here. - Jean II */
+		self->object->continue_received = 1;
+
+		if (self->object->suspend)
+			break;
+
 		ret = obex_object_send(self, self->object, TRUE, TRUE);
 		if (ret == 0) {
 			/* Made some progress */
 			obex_deliver_event(self, OBEX_EV_PROGRESS, cmd, 0, FALSE);
+			self->object->first_packet_sent = 1;
+			self->object->continue_received = 0;
 		} else if (ret < 0) {
 			/* Error sending response */
 			obex_deliver_event(self, OBEX_EV_LINKERR, cmd, 0, TRUE);

Re: [patch] fix the bug of obex_object_resume(), obex_server() in openobex

[Johan Hedberg]

Hi Forrest,

On Dec 23, 2008, at 3:33, Zhao Forrest wrote:
> When developing the PBAP server in obexd we found that
> obex_object_resume() can't work as expected. After debugging openobex
> we post this patch to fix the bug. We did the internal test with PBAP
> server/client. The below patch fixes the bug in obex_object_resume()
> and obex_server().

The patch seems fine and has been pushed upstream, thanks! Btw, in the  
future please use the openobex-users mailing list for openobex  
specific patches.

Johan

[patch] obexd: if path == NULL, don't call dbus_connection_get_object_path_data

[Liu, Raymond]

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

Hi

	Just a minor fix for g_dbus_unregister_interface to avoid calling dbus_connection_get_object_path_data when path is NULL.

Best Regards,
Raymond Liu

[-- Attachment #2: 0001-bug-fix-if-path-NULL-don-t-call-dbus_connection.patch --]
[-- Type: application/octet-stream, Size: 792 bytes --]

From ad7ab3dfe96fed455f39bea2181e71603515ffda Mon Sep 17 00:00:00 2001
From: Raymond Liu <raymond.liu@intel.com>
Date: Wed, 24 Dec 2008 10:54:47 +0800
Subject: [PATCH] bug fix: if path == NULL, don't call dbus_connection_get_object_path_data

---
 gdbus/object.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 432a8a0..1c8bcc4 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -494,7 +494,7 @@ gboolean g_dbus_unregister_interface(DBusConnection *connection,
 	struct generic_data *data = NULL;
 	struct interface_data *iface;
 
-	if (dbus_connection_get_object_path_data(connection, path,
+	if (!path || dbus_connection_get_object_path_data(connection, path,
 						(void *) &data) == FALSE)
 		return FALSE;
 
-- 
1.5.6.3

Re: [patch] obexd: if path == NULL, don't call dbus_connection_get_object_path_data

[Marcel Holtmann]

Hi Raymond,

> 	Just a minor fix for g_dbus_unregister_interface to avoid calling dbus_connection_get_object_path_data when path is NULL.

@@ -494,7 +494,7 @@ gboolean g_dbus_unregister_interface(DBusConnection *connection,
        struct generic_data *data = NULL;
        struct interface_data *iface;
 
-       if (dbus_connection_get_object_path_data(connection, path,
+       if (!path || dbus_connection_get_object_path_data(connection, path,
                                                (void *) &data) == FALSE)
                return FALSE;

So doing some like this would be better:

	if (!path)
		return FALSE;

	if (dbus_connection ...)
		return FALSE;

You should really start thinking for the error => return paradigm all
the time. This makes the code a lot simpler to read.

Regards

Marcel

RE: [patch] obexd: if path == NULL, don't call dbus_connection_get_object_path_data

[Liu, Raymond]

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

Hi Marcel

>
>You should really start thinking for the error => return paradigm all
>the time. This makes the code a lot simpler to read.
>
>Regards
>Marcel
>

OK, Got it. Resent

Raymond


[-- Attachment #2: 0001-bug-fix-if-path-NULL-don-t-call-dbus_connection.patch --]
[-- Type: application/octet-stream, Size: 753 bytes --]

From baea853a2338b65feb68e0103ef8795467cada7d Mon Sep 17 00:00:00 2001
From: Raymond Liu <raymond.liu@intel.com>
Date: Wed, 24 Dec 2008 10:54:47 +0800
Subject: [PATCH] bug fix: if path == NULL, don't call dbus_connection_get_object_path_data

---
 gdbus/object.c |    3 +++
 1 files changed, 3 insertions(+), 0 deletions(-)

diff --git a/gdbus/object.c b/gdbus/object.c
index 432a8a0..a41617d 100644
--- a/gdbus/object.c
+++ b/gdbus/object.c
@@ -494,6 +494,9 @@ gboolean g_dbus_unregister_interface(DBusConnection *connection,
 	struct generic_data *data = NULL;
 	struct interface_data *iface;
 
+	if (!path)
+		return FALSE;
+
 	if (dbus_connection_get_object_path_data(connection, path,
 						(void *) &data) == FALSE)
 		return FALSE;
-- 
1.5.6.3

RE: [patch] obexd: if path == NULL, don't call dbus_connection_get_object_path_data

[Marcel Holtmann]

Hi Raymond,

> >You should really start thinking for the error => return paradigm all
> >the time. This makes the code a lot simpler to read.
>
> OK, Got it. Resent

applied to the bluez.git and obexd.git trees. Thanks.

Regards

Marcel