2:33 a.m.
Hi,
This series concerns mmsd for ofono mailing list.
This is essentially a rework of Ronald's patches.
Was:
Hi,
These patches concern mmsd and are related to error handling while
receiving message.
The main idea is to try to recover error internally when possible and
drop messages that are not decodable.
Ronald Tessier (1):
service: remove files when unable to decode received msg
Sébastien Bianti (9):
service: add debug prints
service: remove useless variable
service: add attempts counter variable
service: move destroy request into process_request
service: refactoring file closure
service: callbacks should return a boolean
service: refactoring error code
service: add requeue functionality
service: bearer activity needs to be checked
src/service.c | 134 ++++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 90 insertions(+), 44 deletions(-)
--
1.7.4.4
2:33 a.m.
New subject: [PATCH v2 01/10] service: remove files when unable to decode received msg
From: Ronald Tessier <ronald.tessier(a)linux.intel.com>
---
src/service.c | 3 +++
1 files changed, 3 insertions(+), 0 deletions(-)
diff --git a/src/service.c b/src/service.c
index f148252..593adcb 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1019,6 +1019,9 @@ out:
mms_store_meta_close(service_id, uuid, meta, FALSE);
+ if (success == FALSE)
+ mms_store_remove(service_id, uuid);
+
return success;
}
--
1.7.4.4
2:33 a.m.
New subject: [PATCH v2 02/10] service: add debug prints
---
src/service.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/src/service.c b/src/service.c
index 593adcb..6041d6d 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1804,8 +1804,10 @@ static gboolean web_get_cb(GWebResult *result, gpointer user_data)
struct mms_service *service;
const guint8 *chunk;
- if (g_web_result_get_chunk(result, &chunk, &chunk_size) == FALSE)
+ if (g_web_result_get_chunk(result, &chunk, &chunk_size) == FALSE) {
+ mms_error("Fail to get data chunk");
goto error;
+ }
if (chunk_size == 0) {
close(request->fd);
@@ -1904,6 +1906,8 @@ static guint process_request(struct mms_request *request)
return id;
}
+ mms_error("Cannot process request (request type: %d)", request->type);
+
unlink(request->data_path);
return 0;
@@ -2020,6 +2024,8 @@ error:
out:
mms_message_free(msg);
+
+ mms_error("Failed to handle incoming notification");
}
void mms_service_bearer_notify(struct mms_service *service, mms_bool_t active,
--
1.7.4.4
2:33 a.m.
New subject: [PATCH v2 03/10] service: remove useless variable
---
src/service.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/service.c b/src/service.c
index 6041d6d..7969988 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1801,7 +1801,6 @@ static gboolean web_get_cb(GWebResult *result, gpointer user_data)
gsize written;
gsize chunk_size;
struct mms_request *request = user_data;
- struct mms_service *service;
const guint8 *chunk;
if (g_web_result_get_chunk(result, &chunk, &chunk_size) == FALSE) {
@@ -1836,16 +1835,14 @@ error:
unlink(request->data_path);
complete:
- service = request->service;
-
if (request->result_cb != NULL)
request->result_cb(request);
mms_request_destroy(request);
- service->current_request_id = 0;
+ request->service->current_request_id = 0;
- process_request_queue(service);
+ process_request_queue(request->service);
return FALSE;
}
--
1.7.4.4
2:33 a.m.
New subject: [PATCH v2 04/10] service: add attempts counter variable
---
src/service.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/service.c b/src/service.c
index 7969988..a4f236b 100644
--- a/src/service.c
+++ b/src/service.c
@@ -55,6 +55,7 @@
#define CONTENT_TYPE_APP_SMIL "Content-Type:
\"application/smil\";charset=utf-8"
#define MAX_ATTACHMENTS_NUMBER 25
+#define MAX_ATTEMPTS 3
#define uninitialized_var(x) x = x
@@ -98,6 +99,7 @@ struct mms_request {
gsize data_size;
int fd;
guint16 status;
+ guint16 attempt;
struct mms_service *service;
mms_request_result_cb_t result_cb;
struct mms_message *msg;
@@ -526,6 +528,8 @@ static struct mms_request *create_request(enum mms_request_type type,
request->status = 0;
+ request->attempt = 0;
+
return request;
}
--
1.7.4.4
2:33 a.m.
New subject: [PATCH v2 05/10] service: move destroy request into process_request
To make it possible to be re-queued
---
src/service.c | 4 ++--
1 files changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/service.c b/src/service.c
index a4f236b..ee06dca 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1911,6 +1911,8 @@ static guint process_request(struct mms_request *request)
unlink(request->data_path);
+ mms_request_destroy(request);
+
return 0;
}
@@ -1938,8 +1940,6 @@ static void process_request_queue(struct mms_service *service)
service->current_request_id = process_request(request);
if (service->current_request_id > 0)
return;
-
- mms_request_destroy(request);
}
service->bearer_timeout = g_timeout_add_seconds(BEARER_IDLE_TIMEOUT,
--
1.7.4.4
2:33 a.m.
New subject: [PATCH v2 06/10] service: refactoring file closure
---
src/service.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/service.c b/src/service.c
index ee06dca..1be0386 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1813,8 +1813,6 @@ static gboolean web_get_cb(GWebResult *result, gpointer user_data)
}
if (chunk_size == 0) {
- close(request->fd);
-
request->status = g_web_result_get_status(result);
DBG("status: %03u", request->status);
@@ -1835,10 +1833,11 @@ static gboolean web_get_cb(GWebResult *result, gpointer
user_data)
return TRUE;
error:
- close(request->fd);
unlink(request->data_path);
complete:
+ close(request->fd);
+
if (request->result_cb != NULL)
request->result_cb(request);
--
1.7.4.4
2:33 a.m.
New subject: [PATCH v2 07/10] service: callbacks should return a boolean
To decide whether retry or not.
---
src/service.c | 41 +++++++++++++++++++++++------------------
1 files changed, 23 insertions(+), 18 deletions(-)
diff --git a/src/service.c b/src/service.c
index 1be0386..f8ad05f 100644
--- a/src/service.c
+++ b/src/service.c
@@ -68,7 +68,7 @@ static const char *sep_chars = "()<>@,;:\\\"/[]?={}
\t";
struct mms_request;
-typedef void (*mms_request_result_cb_t) (struct mms_request *request);
+typedef gboolean (*mms_request_result_cb_t) (struct mms_request *request);
struct mms_service {
gint refcount;
@@ -581,7 +581,7 @@ static inline char *create_transaction_id(void)
"0123456789ABCDEF0123456789ABCDEF");
}
-static void result_request_send_conf(struct mms_request *request)
+static gboolean result_request_send_conf(struct mms_request *request)
{
struct mms_message *msg;
struct mms_service *service = request->service;
@@ -592,15 +592,15 @@ static void result_request_send_conf(struct mms_request *request)
char *path;
if (request->status != 200)
- return;
+ return FALSE;
msg = g_try_new0(struct mms_message, 1);
if (msg == NULL)
- return;
+ return FALSE;
if (mmap_file(request->data_path, &pdu, &len) == FALSE) {
mms_message_free(msg);
- return;
+ return FALSE;
}
if (mms_message_decode(pdu, len, msg) == FALSE) {
@@ -610,7 +610,7 @@ static void result_request_send_conf(struct mms_request *request)
mms_message_free(msg);
- return;
+ return FALSE;
}
mms_debug("response status : %d", msg->sc.rsp_status);
@@ -622,13 +622,13 @@ static void result_request_send_conf(struct mms_request *request)
unlink(request->data_path);
if (request->msg == NULL)
- return;
+ return FALSE;
uuid = request->msg->uuid;
meta = mms_store_meta_open(service->identity, uuid);
if (meta == NULL)
- return;
+ return FALSE;
g_key_file_set_string(meta, "info", "state", "sent");
@@ -639,6 +639,8 @@ static void result_request_send_conf(struct mms_request *request)
emit_msg_status_changed(path, "sent");
g_free(path);
+
+ return TRUE;
}
static void append_message(const char *path, const struct mms_service *service,
@@ -1029,8 +1031,8 @@ out:
return success;
}
-static void result_request_retrieve_conf(struct mms_request *request);
-static void result_request_notify_resp(struct mms_request *request);
+static gboolean result_request_retrieve_conf(struct mms_request *request);
+static gboolean result_request_notify_resp(struct mms_request *request);
static struct mms_request *build_notify_resp_ind(struct mms_service *service,
enum mms_message_notify_status status,
@@ -1693,7 +1695,7 @@ static gboolean bearer_idle_timeout(gpointer user_data)
return FALSE;
}
-static void result_request_notify_resp(struct mms_request *request)
+static gboolean result_request_notify_resp(struct mms_request *request)
{
struct mms_message *msg;
GKeyFile *meta;
@@ -1703,17 +1705,17 @@ static void result_request_notify_resp(struct mms_request
*request)
if (request->status != 200) {
mms_error("POST m.notify.resp.ind failed with status %d",
request->status);
- return;
+ return FALSE;
}
if (request->msg == NULL)
- return;
+ return FALSE;
msg = mms_request_steal_message(request);
if (mms_message_register(request->service, msg) != 0) {
mms_message_free(msg);
- return;
+ return FALSE;
}
emit_message_added(request->service, msg);
@@ -1721,15 +1723,17 @@ static void result_request_notify_resp(struct mms_request
*request)
meta = mms_store_meta_open(request->service->identity,
msg->uuid);
if (meta == NULL)
- return;
+ return FALSE;
g_key_file_set_string(meta, "info", "state", "received");
mms_store_meta_close(request->service->identity,
msg->uuid, meta, TRUE);
+
+ return TRUE;
}
-static void result_request_retrieve_conf(struct mms_request *request)
+static gboolean result_request_retrieve_conf(struct mms_request *request)
{
struct mms_message *msg;
struct mms_service *service = request->service;
@@ -1741,10 +1745,10 @@ static void result_request_retrieve_conf(struct mms_request
*request)
gboolean decode_success;
if (request->status != 200)
- return;
+ return FALSE;
if (mmap_file(request->data_path, &pdu, &len) == FALSE)
- return;
+ return FALSE;
uuid = mms_store_file(service->identity, request->data_path);
if (uuid == NULL)
@@ -1798,6 +1802,7 @@ error:
exit:
munmap(pdu, len);
+ return TRUE;
}
static gboolean web_get_cb(GWebResult *result, gpointer user_data)
--
1.7.4.4
2:33 a.m.
New subject: [PATCH v2 08/10] service: refactoring error code
---
src/service.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/service.c b/src/service.c
index f8ad05f..de92487 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1814,7 +1814,7 @@ static gboolean web_get_cb(GWebResult *result, gpointer user_data)
if (g_web_result_get_chunk(result, &chunk, &chunk_size) == FALSE) {
mms_error("Fail to get data chunk");
- goto error;
+ goto complete;
}
if (chunk_size == 0) {
@@ -1832,21 +1832,21 @@ static gboolean web_get_cb(GWebResult *result, gpointer
user_data)
if (written != chunk_size) {
mms_error("only %zd/%zd bytes written\n", written, chunk_size);
- goto error;
+ goto complete;
}
return TRUE;
-error:
- unlink(request->data_path);
-
complete:
close(request->fd);
- if (request->result_cb != NULL)
- request->result_cb(request);
-
- mms_request_destroy(request);
+ if (request->result_cb == NULL || request->result_cb(request) == TRUE)
+ mms_request_destroy(request);
+ else {
+ mms_error("Fail to get data (http status = %03u)",
+ request->status);
+ unlink(request->data_path);
+ }
request->service->current_request_id = 0;
--
1.7.4.4
2:33 a.m.
New subject: [PATCH v2 09/10] service: add requeue functionality
---
src/service.c | 26 +++++++++++++++++++++++++-
1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/src/service.c b/src/service.c
index de92487..01d9886 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1805,6 +1805,25 @@ exit:
return TRUE;
}
+static gboolean mms_requeue_request(struct mms_request *request)
+{
+ request->attempt += 1;
+
+ if (request->attempt == MAX_ATTEMPTS)
+ return FALSE;
+
+ if (request->type == MMS_REQUEST_TYPE_GET) {
+ request->fd = open(request->data_path, O_WRONLY | O_TRUNC,
+ S_IWUSR | S_IRUSR);
+ if (request->fd < 0)
+ return FALSE;
+ }
+
+ g_queue_push_tail(request->service->request_queue, request);
+
+ return TRUE;
+}
+
static gboolean web_get_cb(GWebResult *result, gpointer user_data)
{
gsize written;
@@ -1845,7 +1864,12 @@ complete:
else {
mms_error("Fail to get data (http status = %03u)",
request->status);
- unlink(request->data_path);
+ if (mms_requeue_request(request) == TRUE)
+ mms_error("retry later");
+ else {
+ unlink(request->data_path);
+ mms_request_destroy(request);
+ }
}
request->service->current_request_id = 0;
--
1.7.4.4
2:33 a.m.
New subject: [PATCH v2 10/10] service: bearer activity needs to be checked
Because failure and requeing reason can be network lost, and we should
wait for recovery.
---
src/service.c | 22 +++++++++++++++-------
1 files changed, 15 insertions(+), 7 deletions(-)
diff --git a/src/service.c b/src/service.c
index 01d9886..550f193 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1660,17 +1660,22 @@ int mms_service_set_bearer_handler(struct mms_service *service,
return 0;
}
-static void deactivate_bearer(struct mms_service *service)
+static inline gboolean bearer_is_active(struct mms_service *service)
{
- DBG("service %p", service);
-
if (service->bearer_setup == TRUE)
- return;
-
- if (service->bearer_active == FALSE)
- return;
+ return FALSE;
if (service->bearer_handler == NULL)
+ return FALSE;
+
+ return service->bearer_active;
+}
+
+static void deactivate_bearer(struct mms_service *service)
+{
+ DBG("service %p", service);
+
+ if (bearer_is_active(service) == FALSE)
return;
DBG("service %p", service);
@@ -1959,6 +1964,9 @@ static void process_request_queue(struct mms_service *service)
return;
while (1) {
+ if (bearer_is_active(service) == FALSE)
+ return;
+
request = g_queue_pop_head(service->request_queue);
if (request == NULL)
break;
--
1.7.4.4
11:26 p.m.
Hi all,
On 04/13/2012 06:33 PM, Sébastien Bianti wrote:
Hi,
This series concerns mmsd for ofono mailing list.
This is essentially a rework of Ronald's patches.
Was:
> Hi,
> These patches concern mmsd and are related to error handling while
> receiving message.
> The main idea is to try to recover error internally when possible and
> drop messages that are not decodable.
Ronald Tessier (1):
service: remove files when unable to decode received msg
Sébastien Bianti (9):
service: add debug prints
service: remove useless variable
service: add attempts counter variable
service: move destroy request into process_request
service: refactoring file closure
service: callbacks should return a boolean
service: refactoring error code
service: add requeue functionality
service: bearer activity needs to be checked
src/service.c | 134 ++++++++++++++++++++++++++++++++++++++-------------------
1 files changed, 90 insertions(+), 44 deletions(-)
Please ignore this set of patches since the first patch has to be
modified. I send an updated version.
Regards
Ronald
3805
days inactive
3810
days old
11 comments
2 participants
participants (2)
-
Ronald Tessier -
Sébastien Bianti