11:27 p.m.
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.
Please, ignore previous set of patches sent by Sebastien (when he
reworked my patches) since these are the same except the first
patch (don't always remove message's files, only when the message
is 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 | 136 ++++++++++++++++++++++++++++++++++++++------------------
1 files changed, 92 insertions(+), 44 deletions(-)
--
1.7.4.1
11:27 p.m.
New subject: [PATCHv3 01/10] service: remove files when unable to decode received msg
Remove files when message is not decodable.
---
src/service.c | 5 +++++
1 files changed, 5 insertions(+), 0 deletions(-)
diff --git a/src/service.c b/src/service.c
index f148252..7f2a302 100644
--- a/src/service.c
+++ b/src/service.c
@@ -961,6 +961,7 @@ static gboolean load_message_from_store(const char *service_id,
gboolean read_status;
char *data_path = NULL;
gboolean success = FALSE;
+ gboolean tainted = FALSE;
void *pdu;
size_t len;
@@ -984,6 +985,7 @@ static gboolean load_message_from_store(const char *service_id,
if (mms_message_decode(pdu, len, msg) == FALSE) {
mms_error("Failed to decode %s", data_path);
munmap(pdu, len);
+ tainted = TRUE;
goto out;
}
@@ -1019,6 +1021,9 @@ out:
mms_store_meta_close(service_id, uuid, meta, FALSE);
+ if (tainted == TRUE)
+ mms_store_remove(service_id, uuid);
+
return success;
}
--
1.7.4.1
11:27 p.m.
New subject: [PATCHv3 02/10] service: add debug prints
From: Sébastien Bianti <sebastien.bianti(a)linux.intel.com>
---
src/service.c | 8 +++++++-
1 files changed, 7 insertions(+), 1 deletions(-)
diff --git a/src/service.c b/src/service.c
index 7f2a302..e36f318 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1806,8 +1806,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);
@@ -1906,6 +1908,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;
@@ -2022,6 +2026,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.1
11:27 p.m.
New subject: [PATCHv3 03/10] service: remove useless variable
From: Sébastien Bianti <sebastien.bianti(a)linux.intel.com>
---
src/service.c | 7 ++-----
1 files changed, 2 insertions(+), 5 deletions(-)
diff --git a/src/service.c b/src/service.c
index e36f318..4ec49e6 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1803,7 +1803,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) {
@@ -1838,16 +1837,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.1
11:27 p.m.
New subject: [PATCHv3 04/10] service: add attempts counter variable
From: Sébastien Bianti <sebastien.bianti(a)linux.intel.com>
---
src/service.c | 4 ++++
1 files changed, 4 insertions(+), 0 deletions(-)
diff --git a/src/service.c b/src/service.c
index 4ec49e6..4c8a87e 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.1
11:27 p.m.
New subject: [PATCHv3 05/10] service: move destroy request into process_request
From: Sébastien Bianti <sebastien.bianti(a)linux.intel.com>
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 4c8a87e..efb90cb 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1913,6 +1913,8 @@ static guint process_request(struct mms_request *request)
unlink(request->data_path);
+ mms_request_destroy(request);
+
return 0;
}
@@ -1940,8 +1942,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.1
11:27 p.m.
New subject: [PATCHv3 06/10] service: refactoring file closure
From: Sébastien Bianti <sebastien.bianti(a)linux.intel.com>
---
src/service.c | 5 ++---
1 files changed, 2 insertions(+), 3 deletions(-)
diff --git a/src/service.c b/src/service.c
index efb90cb..70b3dda 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1815,8 +1815,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);
@@ -1837,10 +1835,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.1
11:27 p.m.
New subject: [PATCHv3 07/10] service: callbacks should return a boolean
From: Sébastien Bianti <sebastien.bianti(a)linux.intel.com>
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 70b3dda..e577198 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,
@@ -1031,8 +1033,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,
@@ -1695,7 +1697,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;
@@ -1705,17 +1707,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);
@@ -1723,15 +1725,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;
@@ -1743,10 +1747,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)
@@ -1800,6 +1804,7 @@ error:
exit:
munmap(pdu, len);
+ return TRUE;
}
static gboolean web_get_cb(GWebResult *result, gpointer user_data)
--
1.7.4.1
11:27 p.m.
New subject: [PATCHv3 08/10] service: refactoring error code
From: Sébastien Bianti <sebastien.bianti(a)linux.intel.com>
---
src/service.c | 18 +++++++++---------
1 files changed, 9 insertions(+), 9 deletions(-)
diff --git a/src/service.c b/src/service.c
index e577198..bcfd9e0 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1816,7 +1816,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) {
@@ -1834,21 +1834,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.1
11:27 p.m.
New subject: [PATCHv3 09/10] service: add requeue functionality
From: Sébastien Bianti <sebastien.bianti(a)linux.intel.com>
---
src/service.c | 26 +++++++++++++++++++++++++-
1 files changed, 25 insertions(+), 1 deletions(-)
diff --git a/src/service.c b/src/service.c
index bcfd9e0..f25e656 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1807,6 +1807,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;
@@ -1847,7 +1866,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.1
11:27 p.m.
New subject: [PATCHv3 10/10] service: bearer activity needs to be checked
From: Sébastien Bianti <sebastien.bianti(a)linux.intel.com>
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 f25e656..907b581 100644
--- a/src/service.c
+++ b/src/service.c
@@ -1662,17 +1662,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);
@@ -1961,6 +1966,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.1
1:30 a.m.
Hi Ronald,
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.
Please, ignore previous set of patches sent by Sebastien (when he
reworked my patches) since these are the same except the first
patch (don't always remove message's files, only when the message
is 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 | 136 ++++++++++++++++++++++++++++++++++++++------------------
1 files changed, 92 insertions(+), 44 deletions(-)
all 10 patches have been applied.
Regards
Marcel
3805
days inactive
3805
days old
11 comments
2 participants
participants (2)
-
Marcel Holtmann -
Ronald Tessier