[PATCH 1/2] modem: add Lockdown property to Modem interface

[PATCH 1/2] modem: add Lockdown property to Modem interface

[Gustavo F. Padovan]

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

Setting Lockdown to TRUE means power down the modem and hold a lock that
only permits the lock's owner power up the modem back. When released
it restores the last state of the modem before holding the lock.
---
 doc/modem-api.txt |    9 ++++
 src/modem.c       |  129 +++++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 138 insertions(+), 0 deletions(-)

diff --git a/doc/modem-api.txt b/doc/modem-api.txt
index b92e53c..45043b0 100644
--- a/doc/modem-api.txt
+++ b/doc/modem-api.txt
@@ -37,6 +37,15 @@ Properties	boolean Powered [readwrite]
 			Boolean representing the rf state of the modem.
 			Online is false in flight mode.
 
+		boolean Lockdown [readwrite]
+
+			Boolean representing the lock state of the modem.
+			Setting it to true, makes the calling application hold
+			the modem lock and power it down. Setting to false
+			makes the it release the modem lock. Only the
+			application that holds the lock can power up the modem.
+			If the the application exits Lockdown is set to false.
+
 		boolean Emergency [readonly, optional, experimental]
 
 			Boolean representing the emergency mode of the
diff --git a/src/modem.c b/src/modem.c
index b334b58..a9a6e87 100644
--- a/src/modem.c
+++ b/src/modem.c
@@ -71,6 +71,9 @@ struct ofono_modem {
 	ofono_bool_t		powered;
 	ofono_bool_t		powered_pending;
 	ofono_bool_t		get_online;
+	ofono_bool_t		lockdown;
+	char			*lock_owner;
+	guint			lock_watch;
 	guint			timeout;
 	ofono_bool_t		online;
 	struct ofono_watchlist	*online_watches;
@@ -583,6 +586,9 @@ void __ofono_modem_append_properties(struct ofono_modem *modem,
 	ofono_dbus_dict_append(dict, "Powered", DBUS_TYPE_BOOLEAN,
 				&modem->powered);
 
+	ofono_dbus_dict_append(dict, "Lockdown", DBUS_TYPE_BOOLEAN,
+				&modem->lockdown);
+
 	devinfo_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_DEVINFO);
 
 	/* We cheat a little here and don't check the registered status */
@@ -686,6 +692,16 @@ static int set_powered(struct ofono_modem *modem, ofono_bool_t powered)
 	return err;
 }
 
+static void lockdown_remove(struct ofono_modem *modem)
+{
+	DBusConnection *conn =ofono_dbus_get_connection();
+
+	g_free(modem->lock_owner);
+	g_dbus_remove_watch(conn, modem->lock_watch);
+	modem->lock_watch = 0;
+	modem->lockdown = FALSE;
+}
+
 static gboolean set_powered_timeout(gpointer user)
 {
 	struct ofono_modem *modem = user;
@@ -712,11 +728,107 @@ static gboolean set_powered_timeout(gpointer user)
 
 		reply = __ofono_error_timed_out(modem->pending);
 		__ofono_dbus_pending_reply(&modem->pending, reply);
+
+		if (modem->lockdown)
+			lockdown_remove(modem);
 	}
 
 	return FALSE;
 }
 
+static void lockdown_disconnect(DBusConnection *conn, void *user_data)
+{
+	struct ofono_modem *modem = user_data;
+
+	DBG("");
+
+	g_free(modem->lock_owner);
+
+	modem->lockdown = FALSE;
+
+	ofono_dbus_signal_property_changed(conn, modem->path,
+					OFONO_MODEM_INTERFACE,
+					"Lockdown", DBUS_TYPE_BOOLEAN,
+					&modem->lockdown);
+}
+
+static DBusMessage *set_property_lockdown(struct ofono_modem *modem,
+					DBusMessage *msg,
+					DBusMessageIter *var)
+{
+	DBusConnection *conn = ofono_dbus_get_connection();
+	ofono_bool_t lockdown;
+	const char *caller;
+	int err;
+
+	if (dbus_message_iter_get_arg_type(var) != DBUS_TYPE_BOOLEAN)
+		return __ofono_error_invalid_args(msg);
+
+	dbus_message_iter_get_basic(var, &lockdown);
+
+	if (modem->pending != NULL)
+		return __ofono_error_busy(msg);
+
+	if (modem->lockdown == lockdown)
+		return dbus_message_new_method_return(msg);
+
+	caller = dbus_message_get_sender(msg);
+
+	if (lockdown) {
+		dbus_bool_t powered;
+
+		modem->lock_owner = g_strdup(caller);
+
+		modem->lock_watch = g_dbus_add_service_watch(conn,
+				modem->lock_owner, NULL,
+				lockdown_disconnect, modem, NULL);
+
+		if (modem->lock_watch == 0) {
+			g_free(modem->lock_owner);
+			return __ofono_error_failed(msg);
+		}
+
+		modem->lockdown = lockdown;
+
+		if (!modem->powered)
+			goto done;
+
+		err = set_powered(modem, FALSE);
+		if (err < 0) {
+			if (err != -EINPROGRESS) {
+				lockdown_remove(modem);
+				return __ofono_error_failed(msg);
+			}
+
+			modem->pending = dbus_message_ref(msg);
+			modem->timeout = g_timeout_add_seconds(20,
+						set_powered_timeout, modem);
+			return NULL;
+		}
+
+		powered = FALSE;
+		ofono_dbus_signal_property_changed(conn, modem->path,
+					OFONO_MODEM_INTERFACE,
+					"Powered", DBUS_TYPE_BOOLEAN,
+					&powered);
+	} else {
+		if (g_strcmp0(caller, modem->lock_owner))
+			return __ofono_error_access_denied(msg);
+
+		lockdown_remove(modem);
+	}
+
+done:
+	g_dbus_send_reply(conn, msg, DBUS_TYPE_INVALID);
+
+	ofono_dbus_signal_property_changed(conn, modem->path,
+					OFONO_MODEM_INTERFACE,
+					"Lockdown", DBUS_TYPE_BOOLEAN,
+					&lockdown);
+
+	return NULL;
+}
+
 static DBusMessage *modem_set_property(DBusConnection *conn,
 					DBusMessage *msg, void *data)
 {
@@ -759,6 +871,9 @@ static DBusMessage *modem_set_property(DBusConnection *conn,
 		if (modem->powered == powered)
 			return dbus_message_new_method_return(msg);
 
+		if (modem->lockdown)
+			return __ofono_error_access_denied(msg);
+
 		err = set_powered(modem, powered);
 		if (err < 0) {
 			if (err != -EINPROGRESS)
@@ -790,6 +905,9 @@ static DBusMessage *modem_set_property(DBusConnection *conn,
 		return NULL;
 	}
 
+	if (g_str_equal(name, "Lockdown"))
+		return set_property_lockdown(modem, msg, &var);
+
 	return __ofono_error_invalid_args(msg);
 }
 
@@ -834,6 +952,12 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered)
 
 	modem->powered = powered;
 
+	if (modem->lockdown)
+		ofono_dbus_signal_property_changed(conn, modem->path,
+					OFONO_MODEM_INTERFACE,
+					"Lockdown", DBUS_TYPE_BOOLEAN,
+					&modem->lockdown);
+
 	if (modem->driver == NULL) {
 		ofono_error("Calling ofono_modem_set_powered on a"
 				"modem with no driver is not valid, "
@@ -1593,6 +1717,11 @@ static void modem_unregister(struct ofono_modem *modem)
 		modem->interface_update = 0;
 	}
 
+	if (modem->lock_watch) {
+		g_dbus_remove_watch(conn, modem->lock_watch);
+		modem->lock_watch = 0;
+	}
+
 	g_dbus_unregister_interface(conn, modem->path, OFONO_MODEM_INTERFACE);
 
 	if (modem->driver && modem->driver->remove)
-- 
1.7.3.2

[PATCH 2/2] modem: Add test script for the Lockdown property

[Gustavo F. Padovan]

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

---
 Makefile.am         |    3 ++-
 test/lockdown-modem |   25 +++++++++++++++++++++++++
 2 files changed, 27 insertions(+), 1 deletions(-)
 create mode 100755 test/lockdown-modem

diff --git a/Makefile.am b/Makefile.am
index a4c47e8..5414cc3 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -421,7 +421,8 @@ test_scripts = test/backtrace \
 		test/test-push-notification \
 		test/test-smart-messaging \
 		test/send-vcard \
-		test/set-tty
+		test/set-tty \
+		test/lockdown-modem
 
 if TEST
 testdir = $(pkglibdir)/test
diff --git a/test/lockdown-modem b/test/lockdown-modem
new file mode 100755
index 0000000..5d98154
--- /dev/null
+++ b/test/lockdown-modem
@@ -0,0 +1,25 @@
+#!/usr/bin/python
+
+import dbus
+import sys
+
+bus = dbus.SystemBus()
+
+if len(sys.argv) == 2:
+	path = sys.argv[1]
+else:
+	manager = dbus.Interface(bus.get_object('org.ofono', '/'),
+			'org.ofono.Manager')
+	modems = manager.GetModems()
+	path = modems[0][0]
+
+print "Locking and disconnecting modem %s..." % path
+modem = dbus.Interface(bus.get_object('org.ofono', path),
+						'org.ofono.Modem')
+
+modem.SetProperty("Lockdown", dbus.Boolean(1))
+
+print "press ENTER to unlock the modem %s" % path
+sys.stdin.readline()
+
+modem.SetProperty("Lockdown", dbus.Boolean(0))
-- 
1.7.3.2

Re: [PATCH 1/2] modem: add Lockdown property to Modem interface

[Denis Kenzior]

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

Hi Gustavo,

On 12/03/2010 03:59 PM, Gustavo F. Padovan wrote:
> Setting Lockdown to TRUE means power down the modem and hold a lock that
> only permits the lock's owner power up the modem back. When released
> it restores the last state of the modem before holding the lock.
> ---
>  doc/modem-api.txt |    9 ++++
>  src/modem.c       |  129 +++++++++++++++++++++++++++++++++++++++++++++++++++++
>  2 files changed, 138 insertions(+), 0 deletions(-)

Can you break this up into two patches, one for doc and one for src?

> 
> diff --git a/doc/modem-api.txt b/doc/modem-api.txt
> index b92e53c..45043b0 100644
> --- a/doc/modem-api.txt
> +++ b/doc/modem-api.txt
> @@ -37,6 +37,15 @@ Properties	boolean Powered [readwrite]
>  			Boolean representing the rf state of the modem.
>  			Online is false in flight mode.
>  
> +		boolean Lockdown [readwrite]
> +
> +			Boolean representing the lock state of the modem.
> +			Setting it to true, makes the calling application hold
> +			the modem lock and power it down. Setting to false
> +			makes the it release the modem lock. Only the
> +			application that holds the lock can power up the modem.
> +			If the the application exits Lockdown is set to false.
> +
>  		boolean Emergency [readonly, optional, experimental]
>  
>  			Boolean representing the emergency mode of the
> diff --git a/src/modem.c b/src/modem.c
> index b334b58..a9a6e87 100644
> --- a/src/modem.c
> +++ b/src/modem.c
> @@ -71,6 +71,9 @@ struct ofono_modem {
>  	ofono_bool_t		powered;
>  	ofono_bool_t		powered_pending;
>  	ofono_bool_t		get_online;
> +	ofono_bool_t		lockdown;
> +	char			*lock_owner;
> +	guint			lock_watch;
>  	guint			timeout;
>  	ofono_bool_t		online;
>  	struct ofono_watchlist	*online_watches;
> @@ -583,6 +586,9 @@ void __ofono_modem_append_properties(struct ofono_modem *modem,
>  	ofono_dbus_dict_append(dict, "Powered", DBUS_TYPE_BOOLEAN,
>  				&modem->powered);
>  
> +	ofono_dbus_dict_append(dict, "Lockdown", DBUS_TYPE_BOOLEAN,
> +				&modem->lockdown);
> +
>  	devinfo_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_DEVINFO);
>  
>  	/* We cheat a little here and don't check the registered status */
> @@ -686,6 +692,16 @@ static int set_powered(struct ofono_modem *modem, ofono_bool_t powered)
>  	return err;
>  }
>  
> +static void lockdown_remove(struct ofono_modem *modem)
> +{
> +	DBusConnection *conn =ofono_dbus_get_connection();
> +
> +	g_free(modem->lock_owner);
> +	g_dbus_remove_watch(conn, modem->lock_watch);
> +	modem->lock_watch = 0;
> +	modem->lockdown = FALSE;
> +}
> +
>  static gboolean set_powered_timeout(gpointer user)
>  {
>  	struct ofono_modem *modem = user;
> @@ -712,11 +728,107 @@ static gboolean set_powered_timeout(gpointer user)
>  
>  		reply = __ofono_error_timed_out(modem->pending);
>  		__ofono_dbus_pending_reply(&modem->pending, reply);
> +
> +		if (modem->lockdown)
> +			lockdown_remove(modem);
>  	}
>  
>  	return FALSE;
>  }
>  
> +static void lockdown_disconnect(DBusConnection *conn, void *user_data)
> +{
> +	struct ofono_modem *modem = user_data;
> +
> +	DBG("");
> +
> +	g_free(modem->lock_owner);
> +
> +	modem->lockdown = FALSE;
> +
> +	ofono_dbus_signal_property_changed(conn, modem->path,
> +					OFONO_MODEM_INTERFACE,
> +					"Lockdown", DBUS_TYPE_BOOLEAN,
> +					&modem->lockdown);

Should you also be removing the lock_watch here?  Since the lock_watch
is no longer relevant and you'll be leaking it otherwise.

> +}
> +
> +static DBusMessage *set_property_lockdown(struct ofono_modem *modem,
> +					DBusMessage *msg,
> +					DBusMessageIter *var)
> +{
> +	DBusConnection *conn = ofono_dbus_get_connection();
> +	ofono_bool_t lockdown;
> +	const char *caller;
> +	int err;
> +
> +	if (dbus_message_iter_get_arg_type(var) != DBUS_TYPE_BOOLEAN)
> +		return __ofono_error_invalid_args(msg);
> +
> +	dbus_message_iter_get_basic(var, &lockdown);
> +
> +	if (modem->pending != NULL)
> +		return __ofono_error_busy(msg);
> +
> +	if (modem->lockdown == lockdown)
> +		return dbus_message_new_method_return(msg);
> +
> +	caller = dbus_message_get_sender(msg);
> +
> +	if (lockdown) {
> +		dbus_bool_t powered;
> +
> +		modem->lock_owner = g_strdup(caller);
> +
> +		modem->lock_watch = g_dbus_add_service_watch(conn,
> +				modem->lock_owner, NULL,
> +				lockdown_disconnect, modem, NULL);
> +
> +		if (modem->lock_watch == 0) {
> +			g_free(modem->lock_owner);
> +			return __ofono_error_failed(msg);
> +		}
> +
> +		modem->lockdown = lockdown;
> +
> +		if (!modem->powered)
> +			goto done;
> +
> +		err = set_powered(modem, FALSE);
> +		if (err < 0) {
> +			if (err != -EINPROGRESS) {
> +				lockdown_remove(modem);
> +				return __ofono_error_failed(msg);
> +			}
> +
> +			modem->pending = dbus_message_ref(msg);
> +			modem->timeout = g_timeout_add_seconds(20,
> +						set_powered_timeout, modem);
> +			return NULL;
> +		}
> +
> +		powered = FALSE;
> +		ofono_dbus_signal_property_changed(conn, modem->path,
> +					OFONO_MODEM_INTERFACE,
> +					"Powered", DBUS_TYPE_BOOLEAN,
> +					&powered);
> +	} else {
> +		if (g_strcmp0(caller, modem->lock_owner))
> +			return __ofono_error_access_denied(msg);
> +
> +		lockdown_remove(modem);
> +	}
> +
> +done:
> +	g_dbus_send_reply(conn, msg, DBUS_TYPE_INVALID);
> +
> +	ofono_dbus_signal_property_changed(conn, modem->path,
> +					OFONO_MODEM_INTERFACE,
> +					"Lockdown", DBUS_TYPE_BOOLEAN,
> +					&lockdown);
> +
> +	return NULL;
> +}
> +
>  static DBusMessage *modem_set_property(DBusConnection *conn,
>  					DBusMessage *msg, void *data)
>  {
> @@ -759,6 +871,9 @@ static DBusMessage *modem_set_property(DBusConnection *conn,
>  		if (modem->powered == powered)
>  			return dbus_message_new_method_return(msg);
>  
> +		if (modem->lockdown)
> +			return __ofono_error_access_denied(msg);
> +
>  		err = set_powered(modem, powered);
>  		if (err < 0) {
>  			if (err != -EINPROGRESS)
> @@ -790,6 +905,9 @@ static DBusMessage *modem_set_property(DBusConnection *conn,
>  		return NULL;
>  	}
>  
> +	if (g_str_equal(name, "Lockdown"))
> +		return set_property_lockdown(modem, msg, &var);
> +
>  	return __ofono_error_invalid_args(msg);
>  }
>  
> @@ -834,6 +952,12 @@ void ofono_modem_set_powered(struct ofono_modem *modem, ofono_bool_t powered)
>  
>  	modem->powered = powered;
>  
> +	if (modem->lockdown)
> +		ofono_dbus_signal_property_changed(conn, modem->path,
> +					OFONO_MODEM_INTERFACE,
> +					"Lockdown", DBUS_TYPE_BOOLEAN,
> +					&modem->lockdown);
> +
>  	if (modem->driver == NULL) {
>  		ofono_error("Calling ofono_modem_set_powered on a"
>  				"modem with no driver is not valid, "
> @@ -1593,6 +1717,11 @@ static void modem_unregister(struct ofono_modem *modem)
>  		modem->interface_update = 0;
>  	}
>  
> +	if (modem->lock_watch) {
> +		g_dbus_remove_watch(conn, modem->lock_watch);
> +		modem->lock_watch = 0;
> +	}
> +

What about signaling the Lockdown to false, and g_freeing the lock_owner?

>  	g_dbus_unregister_interface(conn, modem->path, OFONO_MODEM_INTERFACE);
>  
>  	if (modem->driver && modem->driver->remove)

Regards,
-Denis

Re: [PATCH 1/2] modem: add Lockdown property to Modem interface

[Gustavo F. Padovan]

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

Hi Denis,

* Denis Kenzior <denkenz@gmail.com> [2010-12-17 18:57:45 -0600]:

> Hi Gustavo,
> 
> On 12/03/2010 03:59 PM, Gustavo F. Padovan wrote:
> > Setting Lockdown to TRUE means power down the modem and hold a lock that
> > only permits the lock's owner power up the modem back. When released
> > it restores the last state of the modem before holding the lock.
> > ---
> >  doc/modem-api.txt |    9 ++++
> >  src/modem.c       |  129 +++++++++++++++++++++++++++++++++++++++++++++++++++++
> >  2 files changed, 138 insertions(+), 0 deletions(-)
> 
> Can you break this up into two patches, one for doc and one for src?
> 
> > 
> > diff --git a/doc/modem-api.txt b/doc/modem-api.txt
> > index b92e53c..45043b0 100644
> > --- a/doc/modem-api.txt
> > +++ b/doc/modem-api.txt
> > @@ -37,6 +37,15 @@ Properties	boolean Powered [readwrite]
> >  			Boolean representing the rf state of the modem.
> >  			Online is false in flight mode.
> >  
> > +		boolean Lockdown [readwrite]
> > +
> > +			Boolean representing the lock state of the modem.
> > +			Setting it to true, makes the calling application hold
> > +			the modem lock and power it down. Setting to false
> > +			makes the it release the modem lock. Only the
> > +			application that holds the lock can power up the modem.
> > +			If the the application exits Lockdown is set to false.
> > +
> >  		boolean Emergency [readonly, optional, experimental]
> >  
> >  			Boolean representing the emergency mode of the
> > diff --git a/src/modem.c b/src/modem.c
> > index b334b58..a9a6e87 100644
> > --- a/src/modem.c
> > +++ b/src/modem.c
> > @@ -71,6 +71,9 @@ struct ofono_modem {
> >  	ofono_bool_t		powered;
> >  	ofono_bool_t		powered_pending;
> >  	ofono_bool_t		get_online;
> > +	ofono_bool_t		lockdown;
> > +	char			*lock_owner;
> > +	guint			lock_watch;
> >  	guint			timeout;
> >  	ofono_bool_t		online;
> >  	struct ofono_watchlist	*online_watches;
> > @@ -583,6 +586,9 @@ void __ofono_modem_append_properties(struct ofono_modem *modem,
> >  	ofono_dbus_dict_append(dict, "Powered", DBUS_TYPE_BOOLEAN,
> >  				&modem->powered);
> >  
> > +	ofono_dbus_dict_append(dict, "Lockdown", DBUS_TYPE_BOOLEAN,
> > +				&modem->lockdown);
> > +
> >  	devinfo_atom = __ofono_modem_find_atom(modem, OFONO_ATOM_TYPE_DEVINFO);
> >  
> >  	/* We cheat a little here and don't check the registered status */
> > @@ -686,6 +692,16 @@ static int set_powered(struct ofono_modem *modem, ofono_bool_t powered)
> >  	return err;
> >  }
> >  
> > +static void lockdown_remove(struct ofono_modem *modem)
> > +{
> > +	DBusConnection *conn =ofono_dbus_get_connection();
> > +
> > +	g_free(modem->lock_owner);
> > +	g_dbus_remove_watch(conn, modem->lock_watch);
> > +	modem->lock_watch = 0;
> > +	modem->lockdown = FALSE;
> > +}
> > +
> >  static gboolean set_powered_timeout(gpointer user)
> >  {
> >  	struct ofono_modem *modem = user;
> > @@ -712,11 +728,107 @@ static gboolean set_powered_timeout(gpointer user)
> >  
> >  		reply = __ofono_error_timed_out(modem->pending);
> >  		__ofono_dbus_pending_reply(&modem->pending, reply);
> > +
> > +		if (modem->lockdown)
> > +			lockdown_remove(modem);
> >  	}
> >  
> >  	return FALSE;
> >  }
> >  
> > +static void lockdown_disconnect(DBusConnection *conn, void *user_data)
> > +{
> > +	struct ofono_modem *modem = user_data;
> > +
> > +	DBG("");
> > +
> > +	g_free(modem->lock_owner);
> > +
> > +	modem->lockdown = FALSE;
> > +
> > +	ofono_dbus_signal_property_changed(conn, modem->path,
> > +					OFONO_MODEM_INTERFACE,
> > +					"Lockdown", DBUS_TYPE_BOOLEAN,
> > +					&modem->lockdown);
> 
> Should you also be removing the lock_watch here?  Since the lock_watch
> is no longer relevant and you'll be leaking it otherwise.

gdbus remove the watch for us in this case. So no leak here.

-- 
Gustavo F. Padovan
http://profusion.mobi