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
_______________________________________________
ofono mailing list
ofono@ofono.org
http://lists.ofono.org/listinfo/ofono
