[PATCH 3/4] Fix possible memory leak on SIM reading error.

[PATCH 3/4] Fix possible memory leak on SIM reading error.

[Andrzej Zaborowski]

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

This would also stall the SIM op queue if there's a read error for a record
other than the first.  The other solution would be "goto next;" (keep
reading further records).
---
 src/sim.c |    4 +---
 1 files changed, 1 insertions(+), 3 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index deab6bd..695744f 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -317,9 +317,7 @@ static void sim_op_retrieve_cb(const struct ofono_error *error,
 	int fd;
 
 	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-		if (op->current == 1)
-			sim_op_error(modem);
-
+		sim_op_error(modem);
 		return;
 	}
 
-- 
1.6.1

Re: [PATCH 3/4] Fix possible memory leak on SIM reading error.

[Denis Kenzior]

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

Hi,

> This would also stall the SIM op queue if there's a read error for a record
> other than the first.  The other solution would be "goto next;" (keep
> reading further records).

Patch has been applied.

This really shouldn't happen in practice, but if it does, we should also look 
to see if the read callback functions don't cleanup or don't behave properly 
as a result of an error mid-stream.  It looks like at least sim_msisdn_read_cb 
might be affected.  Could you check?

Regards,
-Denis

Re: [PATCH 3/4] Fix possible memory leak on SIM reading error.

[Andrzej Zaborowski]

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

Hi,

2009/7/31 Denis Kenzior <denkenz@gmail.com>:
> This really shouldn't happen in practice, but if it does, we should also look
> to see if the read callback functions don't cleanup or don't behave properly
> as a result of an error mid-stream.  It looks like at least sim_msisdn_read_cb
> might be affected.  Could you check?

Right, the attached diff adds the needed checks assuming there will be
no more callbacks after an error.

Regards

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Make-sure-SIM-read-callbacks-die-cleanly.patch --]
[-- Type: text/x-patch, Size: 2363 bytes --]

From 1289f437b2934ba14cf757743561fa6bde741e7a Mon Sep 17 00:00:00 2001
From: Andrzej Zaborowski <andrew.zaborowski@intel.com>
Date: Sat, 1 Aug 2009 14:17:43 +0200
Subject: [PATCH] Make sure SIM read callbacks die cleanly.

---
 src/network.c |   12 +++++++++---
 src/sim.c     |    4 ++--
 2 files changed, 11 insertions(+), 5 deletions(-)

diff --git a/src/network.c b/src/network.c
index 8135fd2..33a1f65 100644
--- a/src/network.c
+++ b/src/network.c
@@ -1202,8 +1202,12 @@ static void sim_opl_read_cb(struct ofono_modem *modem, int ok,
 	int total;
 	GSList *l;
 
-	if (!ok)
+	if (!ok) {
+		if (record > 0)
+			goto optimize;
+
 		return;
+	}
 
 	if (structure != OFONO_SIM_FILE_STRUCTURE_FIXED)
 		return;
@@ -1218,6 +1222,7 @@ static void sim_opl_read_cb(struct ofono_modem *modem, int ok,
 	if (record != total)
 		return;
 
+optimize:
 	sim_eons_optimize(netreg->eons);
 
 	for (l = netreg->operator_list; l; l = l->next) {
@@ -1241,7 +1246,7 @@ static void sim_pnn_read_cb(struct ofono_modem *modem, int ok,
 	int total;
 
 	if (!ok)
-		return;
+		goto check;
 
 	if (structure != OFONO_SIM_FILE_STRUCTURE_FIXED)
 		return;
@@ -1256,11 +1261,12 @@ static void sim_pnn_read_cb(struct ofono_modem *modem, int ok,
 
 	sim_eons_add_pnn_record(netreg->eons, record, data, record_length);
 
+check:
 	/* If PNN is not present then OPL is not useful, don't
 	 * retrieve it.  If OPL is not there then PNN[1] will
 	 * still be used for the HPLMN and/or EHPLMN, if PNN
 	 * is present.  */
-	if (record == total && !sim_eons_pnn_is_empty(netreg->eons))
+	if ((record == total || !ok) && !sim_eons_pnn_is_empty(netreg->eons))
 		ofono_sim_read(modem, SIM_EFOPL_FILEID, sim_opl_read_cb, NULL);
 }
 
diff --git a/src/sim.c b/src/sim.c
index 6e144f4..1fcb63e 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -180,7 +180,7 @@ static void sim_msisdn_read_cb(struct ofono_modem *modem, int ok,
 	struct ofono_phone_number *ph;
 
 	if (!ok)
-		return;
+		goto check;
 
 	if (structure != OFONO_SIM_FILE_STRUCTURE_FIXED)
 		return;
@@ -200,7 +200,7 @@ static void sim_msisdn_read_cb(struct ofono_modem *modem, int ok,
 	sim->own_numbers = g_slist_prepend(sim->own_numbers, ph);
 
 check:
-	if (record == total && sim->own_numbers) {
+	if ((record == total || !ok) && sim->own_numbers) {
 		char **own_numbers;
 		DBusConnection *conn = ofono_dbus_get_connection();
 
-- 
1.6.1

Re: [PATCH 3/4] Fix possible memory leak on SIM reading error.

[Denis Kenzior]

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

Hi,

> Right, the attached diff adds the needed checks assuming there will be
> no more callbacks after an error.

Patch has been applied.  Thanks.

Regards,
-Denis