[PATCH v2 0/6] mmsd: bug fixing in send message recovery

[PATCH v2 0/6] mmsd: bug fixing in send message recovery

[=?unknown-8bit?q?S=C3=A9bastien?= Bianti]

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

This series concerns mmsd for ofono mailing list.

Patch 1 to 3 fix some leaks when a problem arises when a send request is
created.
Patch 4 to 6 fix some mistakes in send message recovery.

Sébastien Bianti (6):
  service: remove dead file
  service: fixed possible meta with NULL uuid
  service: remove pdu without meta
  service: request_post_file opens the pdu itself
  service: fix some leaks
  service: request needs to keep msg

 src/service.c |   29 +++++++++++++++++++++++------
 1 files changed, 23 insertions(+), 6 deletions(-)

-- 
1.7.4.4

[PATCH v2 1/6] service: remove dead file

[=?unknown-8bit?q?S=C3=A9bastien?= Bianti]

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

---
 src/service.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)

diff --git a/src/service.c b/src/service.c
index ef6b544..3fdf4e0 100644
--- a/src/service.c
+++ b/src/service.c
@@ -755,6 +755,7 @@ static DBusMessage *send_message(DBusConnection *conn,
 		release_attachement_data(msg->attachments);
 		mms_message_free(msg);
 
+		unlink(request->data_path);
 		mms_request_destroy(request);
 
 		return __mms_error_trans_failure(dbus_msg);
-- 
1.7.4.4

Re: [PATCH v2 1/6] service: remove dead file

[Marcel Holtmann]

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

Hi Sebastien,

> ---
>  src/service.c |    1 +
>  1 files changed, 1 insertions(+), 0 deletions(-)
> 
> diff --git a/src/service.c b/src/service.c
> index ef6b544..3fdf4e0 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -755,6 +755,7 @@ static DBusMessage *send_message(DBusConnection *conn,
>  		release_attachement_data(msg->attachments);
>  		mms_message_free(msg);
>  
> +		unlink(request->data_path);
>  		mms_request_destroy(request);

the commit message makes no sense to me. Why is this a "dead" file now?

Regards

Marcel

[PATCH v2 2/6] service: fixed possible meta with NULL uuid

[=?unknown-8bit?q?S=C3=A9bastien?= Bianti]

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

If mms_store_file fails.
---
 src/service.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/src/service.c b/src/service.c
index 3fdf4e0..0161ec0 100644
--- a/src/service.c
+++ b/src/service.c
@@ -767,6 +767,15 @@ static DBusMessage *send_message(DBusConnection *conn,
 
 	msg->uuid = g_strdup(mms_store_file(service->identity,
 						request->data_path));
+	if (msg->uuid == NULL) {
+		release_attachement_data(msg->attachments);
+		mms_message_free(msg);
+
+		unlink(request->data_path);
+		mms_request_destroy(request);
+
+		return __mms_error_trans_failure(dbus_msg);
+	}
 
 	meta = mms_store_meta_open(service->identity, msg->uuid);
 	if (meta == NULL) {
-- 
1.7.4.4

Re: [PATCH v2 2/6] service: fixed possible meta with NULL uuid

[Marcel Holtmann]

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

Hi Sebastien,

> If mms_store_file fails.
> ---
>  src/service.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
> 
> diff --git a/src/service.c b/src/service.c
> index 3fdf4e0..0161ec0 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -767,6 +767,15 @@ static DBusMessage *send_message(DBusConnection *conn,
>  
>  	msg->uuid = g_strdup(mms_store_file(service->identity,
>  						request->data_path));
> +	if (msg->uuid == NULL) {
> +		release_attachement_data(msg->attachments);
> +		mms_message_free(msg);
> +
> +		unlink(request->data_path);
> +		mms_request_destroy(request);
> +
> +		return __mms_error_trans_failure(dbus_msg);
> +	}

I am seeing a lot of code duplication here. We need to unify this.

Regards

Marcel

[PATCH v2 3/6] service: remove pdu without meta

[=?unknown-8bit?q?S=C3=A9bastien?= Bianti]

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

---
 src/service.c |   11 ++++++-----
 1 files changed, 6 insertions(+), 5 deletions(-)

diff --git a/src/service.c b/src/service.c
index 0161ec0..8dbee12 100644
--- a/src/service.c
+++ b/src/service.c
@@ -777,11 +777,17 @@ static DBusMessage *send_message(DBusConnection *conn,
 		return __mms_error_trans_failure(dbus_msg);
 	}
 
+	g_free(request->data_path);
+
+	request->data_path = g_strdup_printf("%s/.mms/%s/%s", g_get_home_dir(),
+						service->identity, msg->uuid);
+
 	meta = mms_store_meta_open(service->identity, msg->uuid);
 	if (meta == NULL) {
 		release_attachement_data(msg->attachments);
 		mms_message_free(msg);
 
+		unlink(request->data_path);
 		mms_request_destroy(request);
 
 		return __mms_error_trans_failure(dbus_msg);
@@ -804,11 +810,6 @@ static DBusMessage *send_message(DBusConnection *conn,
 
 	release_attachement_data(msg->attachments);
 
-	g_free(request->data_path);
-
-	request->data_path = g_strdup_printf("%s/.mms/%s/%s", g_get_home_dir(),
-						service->identity, msg->uuid);
-
 	g_queue_push_tail(service->request_queue, request);
 
 	activate_bearer(service);
-- 
1.7.4.4

Re: [PATCH v2 3/6] service: remove pdu without meta

[Marcel Holtmann]

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

Hi Sebastien,

>  src/service.c |   11 ++++++-----
>  1 files changed, 6 insertions(+), 5 deletions(-)
> 
> diff --git a/src/service.c b/src/service.c
> index 0161ec0..8dbee12 100644
> --- a/src/service.c
> +++ b/src/service.c
> @@ -777,11 +777,17 @@ static DBusMessage *send_message(DBusConnection *conn,
>  		return __mms_error_trans_failure(dbus_msg);
>  	}
>  
> +	g_free(request->data_path);
> +
> +	request->data_path = g_strdup_printf("%s/.mms/%s/%s", g_get_home_dir(),
> +						service->identity, msg->uuid);
> +
>  	meta = mms_store_meta_open(service->identity, msg->uuid);
>  	if (meta == NULL) {
>  		release_attachement_data(msg->attachments);
>  		mms_message_free(msg);
>  
> +		unlink(request->data_path);
>  		mms_request_destroy(request);

Same here. That code keeps duplicating all over.

Regards

Marcel

[PATCH v2 4/6] service: request_post_file opens the pdu itself

[=?unknown-8bit?q?S=C3=A9bastien?= Bianti]

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

---
 src/service.c |    1 -
 1 files changed, 0 insertions(+), 1 deletions(-)

diff --git a/src/service.c b/src/service.c
index 8dbee12..2047008 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1137,7 +1137,6 @@ static void process_message_on_start(struct mms_service *service,
 
 			request->data_path = mms_store_get_path(service_id,
 									uuid);
-			request->fd = open(request->data_path, O_RDONLY);
 		} else
 			request = NULL;
 register_sr:
-- 
1.7.4.4

Re: [PATCH v2 4/6] service: request_post_file opens the pdu itself

[Marcel Holtmann]

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

Hi Sebastien,

>  src/service.c |    1 -
>  1 files changed, 0 insertions(+), 1 deletions(-)

patch has been applied.

Regards

Marcel

[PATCH v2 5/6] service: fix some leaks

[=?unknown-8bit?q?S=C3=A9bastien?= Bianti]

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

---
 src/service.c |    5 +++++
 1 files changed, 5 insertions(+), 0 deletions(-)

diff --git a/src/service.c b/src/service.c
index 2047008..a68516d 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1135,6 +1135,11 @@ static void process_message_on_start(struct mms_service *service,
 			if (request == NULL)
 				goto register_sr;
 
+			close(request->fd);
+			request->fd = -1;
+
+			unlink(request->data_path);
+			g_free(request->data_path);
 			request->data_path = mms_store_get_path(service_id,
 									uuid);
 		} else
-- 
1.7.4.4

Re: [PATCH v2 5/6] service: fix some leaks

[Marcel Holtmann]

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

Hi Sebastien,

>  src/service.c |    5 +++++
>  1 files changed, 5 insertions(+), 0 deletions(-)

patch has been applied.

Regards

Marcel

[PATCH v2 6/6] service: request needs to keep msg

[=?unknown-8bit?q?S=C3=A9bastien?= Bianti]

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

---
 src/service.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/src/service.c b/src/service.c
index a68516d..824257f 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1142,6 +1142,8 @@ static void process_message_on_start(struct mms_service *service,
 			g_free(request->data_path);
 			request->data_path = mms_store_get_path(service_id,
 									uuid);
+
+			request->msg = msg;
 		} else
 			request = NULL;
 register_sr:
-- 
1.7.4.4

Re: [PATCH v2 6/6] service: request needs to keep msg

[Marcel Holtmann]

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

Hi Sebastien,

>  src/service.c |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)

patch has been applied.

Regards

Marcel