All the mail mirrored from lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sim: Handle multiple invocations of sim_imsi_obtained
@ 2014-05-06 11:54 Slava Monich
  2014-05-06 13:38 ` Denis Kenzior
  0 siblings, 1 reply; 4+ messages in thread
From: Slava Monich @ 2014-05-06 11:54 UTC (permalink / raw
  To: ofono

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

To avoid leaking memory and issuing unnecessary D-Bus signals
---
 src/sim.c | 11 +++++++++--
 1 file changed, 9 insertions(+), 2 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index edae5eb..1b0cb80 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -1425,11 +1425,18 @@ static void sim_set_ready(struct ofono_sim *sim)
 
 static void sim_imsi_obtained(struct ofono_sim *sim, const char *imsi)
 {
-	DBusConnection *conn = ofono_dbus_get_connection();
-	const char *path = __ofono_atom_get_path(sim->atom);
+	DBusConnection *conn;
+	const char *path;
+
+	if (sim->imsi && !strcmp(sim->imsi, imsi))
+		return;
 
+	DBG("%s", imsi);
+	g_free(sim->imsi);
 	sim->imsi = g_strdup(imsi);
 
+	conn = ofono_dbus_get_connection();
+	path = __ofono_atom_get_path(sim->atom);
 	ofono_dbus_signal_property_changed(conn, path,
 						OFONO_SIM_MANAGER_INTERFACE,
 						"SubscriberIdentity",
-- 
1.8.3.2


^ permalink raw reply related	[flat|nested] 4+ messages in thread

* Re: [PATCH] sim: Handle multiple invocations of sim_imsi_obtained
  2014-05-06 11:54 [PATCH] sim: Handle multiple invocations of sim_imsi_obtained Slava Monich
@ 2014-05-06 13:38 ` Denis Kenzior
  2014-05-06 14:22   ` Slava Monich
  0 siblings, 1 reply; 4+ messages in thread
From: Denis Kenzior @ 2014-05-06 13:38 UTC (permalink / raw
  To: ofono

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

Hi Slava,

On 05/06/2014 06:54 AM, Slava Monich wrote:
> To avoid leaking memory and issuing unnecessary D-Bus signals
> ---
>  src/sim.c | 11 +++++++++--
>  1 file changed, 9 insertions(+), 2 deletions(-)
> 
> diff --git a/src/sim.c b/src/sim.c
> index edae5eb..1b0cb80 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -1425,11 +1425,18 @@ static void sim_set_ready(struct ofono_sim *sim)
>  
>  static void sim_imsi_obtained(struct ofono_sim *sim, const char *imsi)
>  {
> -	DBusConnection *conn = ofono_dbus_get_connection();
> -	const char *path = __ofono_atom_get_path(sim->atom);
> +	DBusConnection *conn;
> +	const char *path;
> +
> +	if (sim->imsi && !strcmp(sim->imsi, imsi))
> +		return;

Calling this function a second time is an error.  So this condition
looks highly suspect.  What issue are you trying to address?

>  
> +	DBG("%s", imsi);
> +	g_free(sim->imsi);
>  	sim->imsi = g_strdup(imsi);
>  
> +	conn = ofono_dbus_get_connection();
> +	path = __ofono_atom_get_path(sim->atom);
>  	ofono_dbus_signal_property_changed(conn, path,
>  						OFONO_SIM_MANAGER_INTERFACE,
>  						"SubscriberIdentity",
> 

Regards,
-Denis

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sim: Handle multiple invocations of sim_imsi_obtained
  2014-05-06 13:38 ` Denis Kenzior
@ 2014-05-06 14:22   ` Slava Monich
  2014-05-06 20:59     ` Denis Kenzior
  0 siblings, 1 reply; 4+ messages in thread
From: Slava Monich @ 2014-05-06 14:22 UTC (permalink / raw
  To: ofono

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

Hi Denis,

>> To avoid leaking memory and issuing unnecessary D-Bus signals
>> ---
>>   src/sim.c | 11 +++++++++--
>>   1 file changed, 9 insertions(+), 2 deletions(-)
>>
>> diff --git a/src/sim.c b/src/sim.c
>> index edae5eb..1b0cb80 100644
>> --- a/src/sim.c
>> +++ b/src/sim.c
>> @@ -1425,11 +1425,18 @@ static void sim_set_ready(struct ofono_sim *sim)
>>   
>>   static void sim_imsi_obtained(struct ofono_sim *sim, const char *imsi)
>>   {
>> -	DBusConnection *conn = ofono_dbus_get_connection();
>> -	const char *path = __ofono_atom_get_path(sim->atom);
>> +	DBusConnection *conn;
>> +	const char *path;
>> +
>> +	if (sim->imsi && !strcmp(sim->imsi, imsi))
>> +		return;
> Calling this function a second time is an error.  So this condition
> looks highly suspect.  What issue are you trying to address?

sim_imsi_obtained() is invoked from two places: sim_imsi_cb() and 
sim_efimsi_cb(). sim_imsi_cb() in turn is a completion for 
sim_retrieve_imsi() which is invoked from 5 other asynchronous 
callbacks, mostly triggered by a number of ofono_sim_read() calls. I 
don't claim that I fully understand what's going on and I haven't 
analyzed all the call sequences but it surely is invoked twice in our 
setup. Any suggestions on where the problem might be?

Thanks,
-Slava

^ permalink raw reply	[flat|nested] 4+ messages in thread

* Re: [PATCH] sim: Handle multiple invocations of sim_imsi_obtained
  2014-05-06 14:22   ` Slava Monich
@ 2014-05-06 20:59     ` Denis Kenzior
  0 siblings, 0 replies; 4+ messages in thread
From: Denis Kenzior @ 2014-05-06 20:59 UTC (permalink / raw
  To: ofono

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

Hi Slava,

>>> +    if (sim->imsi && !strcmp(sim->imsi, imsi))
>>> +        return;
>> Calling this function a second time is an error.  So this condition
>> looks highly suspect.  What issue are you trying to address?
> 
> sim_imsi_obtained() is invoked from two places: sim_imsi_cb() and
> sim_efimsi_cb(). sim_imsi_cb() in turn is a completion for

Only one of those functions is called, depending on how the driver
implements obtaining of the IMSI.  Some drivers do not have the ability
to read IMSI using the READ BINARY / READ RECORD SIM commands.

> sim_retrieve_imsi() which is invoked from 5 other asynchronous
> callbacks, mostly triggered by a number of ofono_sim_read() calls. I
> don't claim that I fully understand what's going on and I haven't
> analyzed all the call sequences but it surely is invoked twice in our
> setup. Any suggestions on where the problem might be?

The code flow of the functions deals with Barred / Fixed Dialing Numbers
and the variations between 2G and 3G sims.  The logic is set up in such
a way that sim_retrieve_imsi is called only once.  Looking at the code,
there doesn't seem to be any way for sim_retrieve_imsi to be called twice.

If this is indeed happening, then please let me know under what
circumstances this happens and I can help you track this issue down.

Regards,
-Denis


^ permalink raw reply	[flat|nested] 4+ messages in thread

end of thread, other threads:[~2014-05-06 20:59 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2014-05-06 11:54 [PATCH] sim: Handle multiple invocations of sim_imsi_obtained Slava Monich
2014-05-06 13:38 ` Denis Kenzior
2014-05-06 14:22   ` Slava Monich
2014-05-06 20:59     ` Denis Kenzior

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.