All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH] sim: Query the status of SC facility lock
@ 2016-07-06  8:34 Slava Monich
  2016-07-06 15:38 ` Denis Kenzior
  0 siblings, 1 reply; 4+ messages in thread
From: Slava Monich @ 2016-07-06  8:34 UTC (permalink / raw)
  To: ofono

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

Otherwise "pin" entry in LockedPins may be lost after ofono is restarted.
---
 src/sim.c | 80 +++++++++++++++++++++++++++++----------------------------------
 1 file changed, 37 insertions(+), 43 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index aedc617..634acfc 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -2449,58 +2449,47 @@ static void sim_free_state(struct ofono_sim *sim)
 	sim_free_main_state(sim);
 }
 
-static void sim_query_fac_imsilock_cb(const struct ofono_error *error,
-				ofono_bool_t status,
-				void *data)
+static void sim_set_locked_pin(struct ofono_sim *sim,
+			enum ofono_sim_password_type type, gboolean locked)
 {
-	struct ofono_sim *sim = data;
-	DBusConnection *conn = ofono_dbus_get_connection();
-	const char *path = __ofono_atom_get_path(sim->atom);
-	char **locked_pins;
+	if (sim->locked_pins[type] != locked) {
+		char **locked_pins;
 
-	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-		ofono_error("Querying Facility Lock for IMSI Lock failed");
-		return;
-	}
+		sim->locked_pins[type] = locked;
+		locked_pins = get_locked_pins(sim);
 
-	sim->locked_pins[OFONO_SIM_PASSWORD_PHSIM_PIN] = status;
+		ofono_dbus_signal_array_property_changed(
+				ofono_dbus_get_connection(),
+				__ofono_atom_get_path(sim->atom),
+				OFONO_SIM_MANAGER_INTERFACE, "LockedPins",
+				DBUS_TYPE_STRING, &locked_pins);
 
-	locked_pins = get_locked_pins(sim);
-
-	ofono_dbus_signal_array_property_changed(conn,
-				path,
-				OFONO_SIM_MANAGER_INTERFACE,
-				"LockedPins", DBUS_TYPE_STRING,
-				&locked_pins);
+		g_strfreev(locked_pins);
+	}
+}
 
-	g_strfreev(locked_pins);
+static void sim_query_fac_imsilock_cb(const struct ofono_error *error,
+				ofono_bool_t status, void *data)
+{
+	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
+		sim_set_locked_pin(data, OFONO_SIM_PASSWORD_PHSIM_PIN, status);
+	}
 }
 
 static void sim_query_fac_networklock_cb(const struct ofono_error *error,
-				ofono_bool_t status,
-				void *data)
+				ofono_bool_t status, void *data)
 {
-	struct ofono_sim *sim = data;
-	DBusConnection *conn = ofono_dbus_get_connection();
-	const char *path = __ofono_atom_get_path(sim->atom);
-	char **locked_pins;
-
-	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-		ofono_error("Querying Facility Lock for Network Lock failed");
-		return;
+	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
+		sim_set_locked_pin(data, OFONO_SIM_PASSWORD_PHNET_PIN, status);
 	}
+}
 
-	sim->locked_pins[OFONO_SIM_PASSWORD_PHNET_PIN] = status;
-
-	locked_pins = get_locked_pins(sim);
-
-	ofono_dbus_signal_array_property_changed(conn,
-				path,
-				OFONO_SIM_MANAGER_INTERFACE,
-				"LockedPins", DBUS_TYPE_STRING,
-				&locked_pins);
-
-	g_strfreev(locked_pins);
+static void sim_query_fac_pinlock_cb(const struct ofono_error *error,
+				ofono_bool_t status, void *data)
+{
+	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
+		sim_set_locked_pin(data, OFONO_SIM_PASSWORD_SIM_PIN, status);
+	}
 }
 
 void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
@@ -2529,14 +2518,19 @@ void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
 	call_state_watches(sim);
 
 	if (inserted) {
-		sim->driver->query_facility_lock(sim,
+		if (sim->driver->query_facility_lock) {
+			sim->driver->query_facility_lock(sim,
 					OFONO_SIM_PASSWORD_PHSIM_PIN,
 					sim_query_fac_imsilock_cb, sim);
 
-		sim->driver->query_facility_lock(sim,
+			sim->driver->query_facility_lock(sim,
 					OFONO_SIM_PASSWORD_PHNET_PIN,
 					sim_query_fac_networklock_cb, sim);
 
+			sim->driver->query_facility_lock(sim,
+					OFONO_SIM_PASSWORD_SIM_PIN,
+					sim_query_fac_pinlock_cb, sim);
+		}
 		sim_initialize(sim);
 	} else {
 		sim->pin_type = OFONO_SIM_PASSWORD_NONE;
-- 
1.9.1


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

* Re: [PATCH] sim: Query the status of SC facility lock
  2016-07-06  8:34 Slava Monich
@ 2016-07-06 15:38 ` Denis Kenzior
  0 siblings, 0 replies; 4+ messages in thread
From: Denis Kenzior @ 2016-07-06 15:38 UTC (permalink / raw)
  To: ofono

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

Hi Slava,

On 07/06/2016 03:34 AM, Slava Monich wrote:
> Otherwise "pin" entry in LockedPins may be lost after ofono is restarted.
> ---
>   src/sim.c | 80 +++++++++++++++++++++++++++++----------------------------------
>   1 file changed, 37 insertions(+), 43 deletions(-)
>
> diff --git a/src/sim.c b/src/sim.c
> index aedc617..634acfc 100644
> --- a/src/sim.c
> +++ b/src/sim.c
> @@ -2449,58 +2449,47 @@ static void sim_free_state(struct ofono_sim *sim)
>   	sim_free_main_state(sim);
>   }
>
> -static void sim_query_fac_imsilock_cb(const struct ofono_error *error,
> -				ofono_bool_t status,
> -				void *data)
> +static void sim_set_locked_pin(struct ofono_sim *sim,
> +			enum ofono_sim_password_type type, gboolean locked)
>   {
> -	struct ofono_sim *sim = data;
> -	DBusConnection *conn = ofono_dbus_get_connection();
> -	const char *path = __ofono_atom_get_path(sim->atom);
> -	char **locked_pins;
> +	if (sim->locked_pins[type] != locked) {
> +		char **locked_pins;
>

I'd prefer:

if (sim->locked_pins[type] == locked)
	return;

.. do stuff ..

> -	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> -		ofono_error("Querying Facility Lock for IMSI Lock failed");
> -		return;
> -	}
> +		sim->locked_pins[type] = locked;
> +		locked_pins = get_locked_pins(sim);
>
> -	sim->locked_pins[OFONO_SIM_PASSWORD_PHSIM_PIN] = status;
> +		ofono_dbus_signal_array_property_changed(
> +				ofono_dbus_get_connection(),
> +				__ofono_atom_get_path(sim->atom),
> +				OFONO_SIM_MANAGER_INTERFACE, "LockedPins",
> +				DBUS_TYPE_STRING, &locked_pins);
>
> -	locked_pins = get_locked_pins(sim);
> -
> -	ofono_dbus_signal_array_property_changed(conn,
> -				path,
> -				OFONO_SIM_MANAGER_INTERFACE,
> -				"LockedPins", DBUS_TYPE_STRING,
> -				&locked_pins);
> +		g_strfreev(locked_pins);
> +	}
> +}
>
> -	g_strfreev(locked_pins);
> +static void sim_query_fac_imsilock_cb(const struct ofono_error *error,
> +				ofono_bool_t status, void *data)
> +{
> +	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
> +		sim_set_locked_pin(data, OFONO_SIM_PASSWORD_PHSIM_PIN, status);
> +	}

This is not in-line with the Linux Kernel coding style.  Also, I'd prefer:

if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
	return;

sim_set_locked_pin...

>   }
>
>   static void sim_query_fac_networklock_cb(const struct ofono_error *error,
> -				ofono_bool_t status,
> -				void *data)
> +				ofono_bool_t status, void *data)
>   {
> -	struct ofono_sim *sim = data;
> -	DBusConnection *conn = ofono_dbus_get_connection();
> -	const char *path = __ofono_atom_get_path(sim->atom);
> -	char **locked_pins;
> -
> -	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
> -		ofono_error("Querying Facility Lock for Network Lock failed");
> -		return;
> +	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
> +		sim_set_locked_pin(data, OFONO_SIM_PASSWORD_PHNET_PIN, status);
>   	}
> +}

as above

>
> -	sim->locked_pins[OFONO_SIM_PASSWORD_PHNET_PIN] = status;
> -
> -	locked_pins = get_locked_pins(sim);
> -
> -	ofono_dbus_signal_array_property_changed(conn,
> -				path,
> -				OFONO_SIM_MANAGER_INTERFACE,
> -				"LockedPins", DBUS_TYPE_STRING,
> -				&locked_pins);
> -
> -	g_strfreev(locked_pins);
> +static void sim_query_fac_pinlock_cb(const struct ofono_error *error,
> +				ofono_bool_t status, void *data)
> +{
> +	if (error->type == OFONO_ERROR_TYPE_NO_ERROR) {
> +		sim_set_locked_pin(data, OFONO_SIM_PASSWORD_SIM_PIN, status);
> +	}

as above

>   }
>
>   void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
> @@ -2529,14 +2518,19 @@ void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
>   	call_state_watches(sim);
>
>   	if (inserted) {
> -		sim->driver->query_facility_lock(sim,
> +		if (sim->driver->query_facility_lock) {
> +			sim->driver->query_facility_lock(sim,
>   					OFONO_SIM_PASSWORD_PHSIM_PIN,
>   					sim_query_fac_imsilock_cb, sim);
>
> -		sim->driver->query_facility_lock(sim,
> +			sim->driver->query_facility_lock(sim,
>   					OFONO_SIM_PASSWORD_PHNET_PIN,
>   					sim_query_fac_networklock_cb, sim);
>
> +			sim->driver->query_facility_lock(sim,
> +					OFONO_SIM_PASSWORD_SIM_PIN,
> +					sim_query_fac_pinlock_cb, sim);
> +		}
>   		sim_initialize(sim);

Item M1 in doc/coding-style.txt.

>   	} else {
>   		sim->pin_type = OFONO_SIM_PASSWORD_NONE;
>

Regards,
-Denis

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

* [PATCH] sim: Query the status of SC facility lock
@ 2016-07-06 16:02 Slava Monich
  2016-07-06 16:13 ` Denis Kenzior
  0 siblings, 1 reply; 4+ messages in thread
From: Slava Monich @ 2016-07-06 16:02 UTC (permalink / raw)
  To: ofono

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

Resending after adjusting code style.
---
 src/sim.c | 72 +++++++++++++++++++++++++++++++--------------------------------
 1 file changed, 35 insertions(+), 37 deletions(-)

diff --git a/src/sim.c b/src/sim.c
index aedc617..e2da78d 100644
--- a/src/sim.c
+++ b/src/sim.c
@@ -2449,58 +2449,50 @@ static void sim_free_state(struct ofono_sim *sim)
 	sim_free_main_state(sim);
 }
 
-static void sim_query_fac_imsilock_cb(const struct ofono_error *error,
-				ofono_bool_t status,
-				void *data)
+static void sim_set_locked_pin(struct ofono_sim *sim,
+			enum ofono_sim_password_type type, gboolean locked)
 {
-	struct ofono_sim *sim = data;
-	DBusConnection *conn = ofono_dbus_get_connection();
-	const char *path = __ofono_atom_get_path(sim->atom);
 	char **locked_pins;
 
-	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-		ofono_error("Querying Facility Lock for IMSI Lock failed");
+	if (sim->locked_pins[type] == locked)
 		return;
-	}
-
-	sim->locked_pins[OFONO_SIM_PASSWORD_PHSIM_PIN] = status;
 
+	sim->locked_pins[type] = locked;
 	locked_pins = get_locked_pins(sim);
 
-	ofono_dbus_signal_array_property_changed(conn,
-				path,
-				OFONO_SIM_MANAGER_INTERFACE,
-				"LockedPins", DBUS_TYPE_STRING,
-				&locked_pins);
+	ofono_dbus_signal_array_property_changed(ofono_dbus_get_connection(),
+				__ofono_atom_get_path(sim->atom),
+				OFONO_SIM_MANAGER_INTERFACE, "LockedPins",
+				DBUS_TYPE_STRING, &locked_pins);
 
 	g_strfreev(locked_pins);
 }
 
-static void sim_query_fac_networklock_cb(const struct ofono_error *error,
-				ofono_bool_t status,
-				void *data)
+static void sim_query_fac_imsilock_cb(const struct ofono_error *error,
+				ofono_bool_t status, void *data)
 {
-	struct ofono_sim *sim = data;
-	DBusConnection *conn = ofono_dbus_get_connection();
-	const char *path = __ofono_atom_get_path(sim->atom);
-	char **locked_pins;
-
-	if (error->type != OFONO_ERROR_TYPE_NO_ERROR) {
-		ofono_error("Querying Facility Lock for Network Lock failed");
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
 		return;
-	}
 
-	sim->locked_pins[OFONO_SIM_PASSWORD_PHNET_PIN] = status;
+	sim_set_locked_pin(data, OFONO_SIM_PASSWORD_PHSIM_PIN, status);
+}
 
-	locked_pins = get_locked_pins(sim);
+static void sim_query_fac_networklock_cb(const struct ofono_error *error,
+				ofono_bool_t status, void *data)
+{
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		return;
 
-	ofono_dbus_signal_array_property_changed(conn,
-				path,
-				OFONO_SIM_MANAGER_INTERFACE,
-				"LockedPins", DBUS_TYPE_STRING,
-				&locked_pins);
+	sim_set_locked_pin(data, OFONO_SIM_PASSWORD_PHNET_PIN, status);
+}
 
-	g_strfreev(locked_pins);
+static void sim_query_fac_pinlock_cb(const struct ofono_error *error,
+				ofono_bool_t status, void *data)
+{
+	if (error->type != OFONO_ERROR_TYPE_NO_ERROR)
+		return;
+
+	sim_set_locked_pin(data, OFONO_SIM_PASSWORD_SIM_PIN, status);
 }
 
 void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
@@ -2529,14 +2521,20 @@ void ofono_sim_inserted_notify(struct ofono_sim *sim, ofono_bool_t inserted)
 	call_state_watches(sim);
 
 	if (inserted) {
-		sim->driver->query_facility_lock(sim,
+		if (sim->driver->query_facility_lock) {
+			sim->driver->query_facility_lock(sim,
 					OFONO_SIM_PASSWORD_PHSIM_PIN,
 					sim_query_fac_imsilock_cb, sim);
 
-		sim->driver->query_facility_lock(sim,
+			sim->driver->query_facility_lock(sim,
 					OFONO_SIM_PASSWORD_PHNET_PIN,
 					sim_query_fac_networklock_cb, sim);
 
+			sim->driver->query_facility_lock(sim,
+					OFONO_SIM_PASSWORD_SIM_PIN,
+					sim_query_fac_pinlock_cb, sim);
+		}
+
 		sim_initialize(sim);
 	} else {
 		sim->pin_type = OFONO_SIM_PASSWORD_NONE;
-- 
1.9.1


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

* Re: [PATCH] sim: Query the status of SC facility lock
  2016-07-06 16:02 [PATCH] sim: Query the status of SC facility lock Slava Monich
@ 2016-07-06 16:13 ` Denis Kenzior
  0 siblings, 0 replies; 4+ messages in thread
From: Denis Kenzior @ 2016-07-06 16:13 UTC (permalink / raw)
  To: ofono

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

Hi Slava,

On 07/06/2016 11:02 AM, Slava Monich wrote:
> Resending after adjusting code style.
> ---
>   src/sim.c | 72 +++++++++++++++++++++++++++++++--------------------------------
>   1 file changed, 35 insertions(+), 37 deletions(-)
>

Applied, thanks.

Regards,
-Denis


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

end of thread, other threads:[~2016-07-06 16:13 UTC | newest]

Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2016-07-06 16:02 [PATCH] sim: Query the status of SC facility lock Slava Monich
2016-07-06 16:13 ` Denis Kenzior
  -- strict thread matches above, loose matches on Subject: below --
2016-07-06  8:34 Slava Monich
2016-07-06 15:38 ` 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.