Re: Unable to see cellular service in connman
Hi, On Thu, 2012-10-18 at 15:34 +0100, alok barsode wrote: > AccessPointName = According to the discussion on #connman yesterday, the problem was solved by setting AccessPointName, right? Cheers, Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Connman upstream test result_20121019
Hi all, This is test report for connman-1.8.27.gd10e680-1.1.i586. In this testing,we ran 113 cases. 110 cases passed and 3 cases failed because of known bugs. The pass rate is 97%. We found 1 new bug, reopen 0 bug and verify 0 bugs. In this commit we found the bug that Connmand crashes after enter offlinemode. New bug === 25757 - Connmand crashes after enter offlinemode https://bugs.meego.com/show_bug.cgi?id=25757 Re-open bugs: === N/A Verified bugs: === N/A reproduced bugs: === 25725 - Fail to autoconnect to 3G service after replug in 3G dongle https://bugs.meego.com/show_bug.cgi?id=25725 === Testing Environment == Hardware: netbook Eeepc 1005PE / Cedar Trail / Ocktrail Green Ridge Image: meego-netbook-ia32-1.2.0 / netbook-ia32-pinetrail-tizen_20120424.2 ConnMan: connman-1.8.27.gd10e680-1.1.i586 wpa_supplicant: wpa_supplicant-1.0-13.1.i586 Ofono: ofono-1.11.115.g4769c59-1.1.i586 bluez: bluez-4.87-1.12.i586 3G: Unicom Sim card/ZTE MF190/HuaWei E261 BT: Palm Pre/AnyCom AP/CSR dongle (disable the onboard BT device) Wireless AP:netgear WNR2000/dlink Test Execution Summary == Category Total PASS FAILN/A Comments WiFi17 17 bt 6 6 3G29 28 1 #25725 Ethernet 28 26 2 #25757 Regulatory1 1 NTP 1 1 Openvpn 7 7 surfing3 3 proxy 11 11 tethering 10 10 Total 113110 3 Bug referred here is bug at bugs.meego.com, for example, bug #23110 refer to http://bugs.meego.com/show_bug.cgi?id=23110 ConnMan Test Report List Date:2012-10-19 ---Ethernet--- CM_Eth_BigBroadcastPing PASS CM_Eth_Conn PASS CM_Eth_HasDHCP PASS CM_Eth_IsReEnableWorks PASS CM_Eth_PoweredOnPASS CM_Eth_SmallBroadcastPing PASS CM_Eth_IsPoweredOn PASS CM_Eth_IsReady PASS CM_Eth_Disconnect PASS CM_Eth_PoweredOff PASS CM_Eth_Hotplug PASS CM_Eth_HasIPPASS CM_Eth_Connect PASS CM_Advance_StaticIP_Wired PASS CM_Advance_StaticIP PASS CM_Eth_Upload PASS CM_Eth_Download PASS CM_Flt_eth_autoconn FAIL #25757 CM_Flt_ConnWiredFAIL #25757 CM_Flt_EthNoPingPASS CM_Flt_EthPoweredOffPASS CM_Flt_EthIsPoweredOff PASS CM_Flt_EthPoweredOn PASS CM_Flt_EthOnlinePASS CM_IPv6_eth_set_address PASS CM_Info_EthConnInfo PASS CM_IPv6_eth_ssh PASS CM_AutoConn_RebootWired PASS Total:28 Pass:26 Fail:2 N/A:0 ---WiFi--- CM_WF_wps_pin PASS CM_WF_wps_pbc PASS CM_WS_WEP_UploadPASS CM_WS_WEP_Download PASS CM_WS_PSK2 PASS CM_WS_RSN PASS CM_WS_WEP64 PASS CM_WS_SharedHiddenWEP128PASS CM_WS_WPA PASS CM_WS_OpenBroadcastWEP40PASS CM_WS_WEP128PASS CM_WS_Open PASS CM_WS_HiddenPASS CM_AutoConn_RememWEP64 PASS CM_Autoconn_HIDDEN_Rem_WPAPSKAESPASS CM_Autoconn_RememWPAPSKTKIP PASS CM_Autoconn_HIDDEN_Rem_WEP128 PASS Total:17 Pass:17 Fail:0 N/A:0 ---Bluetooth--- CM_BT_Connect PASS CM_BT_SmallPing PASS CM_BT_BigPing PASS CM_BT_IsReEnableWorks PASS CM_BT_Connect_PAN PASS CM_Advance_StaticIP_BT PASS Total:6 Pass:6 Fail:0 N/A:0 ---Regulatory--- CM_Advance_Regulatory PASS Total:1 Pass:1 Fail:0 N/A:0 ---NTP--- CM_Advance_NTP PASS Total:1 Pass:1 Fail:0 N/A:0 ---3G--- CM_Advance_3G_WiFi_Handover PASS CM_State_3G_Available PASS CM_Flt_Conn3G PASS CM_Flt_3G_autoconn PASS CM_AutoConn_Reboot3GPASS CM_AutoConn_SuspendMem3G
[PATCH] network: Fix typo in debug print
---
src/network.c | 2 +-
1 file changed, 1 insertion(+), 1 deletion(-)
diff --git a/src/network.c b/src/network.c
index 69cb70a..5c66bbd 100644
--- a/src/network.c
+++ b/src/network.c
@@ -719,7 +719,7 @@ void connman_network_set_ipv6_method(struct connman_network
*network,
void connman_network_set_error(struct connman_network *network,
enum connman_network_error error)
{
- DBG("nework %p, error %d", network, error);
+ DBG("network %p error %d", network, error);
network->connecting = FALSE;
network->associating = FALSE;
--
1.7.11.4
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH 0/5] Resolver fixes
On Thu, 2012-10-18 at 14:35 +0300, Jukka Rissanen wrote: > Hi, > > this patchset fixes the latest crash bug 25757. > The patches #1, #2 and #5 add more useful debugging > information. > Patch #3 fixes the crash that was reported. The crash > happened because we received data from DNS server after > we had already freed the data that was needed by callback > function. > Patch #4 fixes potential free memory access by doing the > query destroy and removal of it from the queue differently. All patches applied, thanks a lot! Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH] network: Fix typo in debug print
On Fri, 2012-10-19 at 10:44 +0300, Jukka Rissanen wrote: > --- > src/network.c | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) ...and applied. Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
RE: [PATCH] wifi: Don't set scanning to FALSE during connecting
Hi Tomasz, >-Original Message- >From: connman-boun...@connman.net >[mailto:connman-boun...@connman.net] On Behalf Of Tomasz Bursztyka >Sent: Thursday, October 18, 2012 9:04 PM >To: connman@connman.net >Subject: Re: [PATCH] wifi: Don't set scanning to FALSE during connecting > >Hi Arron, > >Do you have any logs about that? I will send to you directly to avoid information leak:) >This has been here for quite long time and afaik, nothing like that has been >reported. Yes, this is added after ConnMan 1.0 >setting scanning to false removes only unavailable network, so the ones which >has not been found during scanning (set scanning to true puts all network >unavailable, and depending on scan results network comes back, or not, >available) Actually, when associating or completed: scanning should already >be >at FALSE (unless autoscan fallback has restarted yes) > >So you might have found an issue, but I would like more backgrounds about it. Yes, I found this issue on a ARM phone Part of the log: connmand[5710]: plugins/wifi.c:network_connect() network 0xa1938 connmand[5710]: src/network.c:connman_network_get_blob() network 0xa1938 key WiFi.SSID connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.Security connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.Passphrase connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.AgentPassphrase connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.EAP connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.PrivateKeyPassphrase connmand[5710]: src/network.c:connman_network_set_string() network 0xa1938 key WiFi.PrivateKeyPassphrase value (null) connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.Identity connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.AgentIdentity connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.CACertFile connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.ClientCertFile connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.PrivateKeyFile connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.PrivateKeyPassphrase connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.Phase2 connmand[5710]: src/network.c:connman_network_get_bool() network 0xa1938 key WiFi.UseWPS connmand[5710]: src/network.c:connman_network_get_string() network 0xa1938 key WiFi.PinWPS connmand[5710]: src/network.c:connman_network_set_associating() network 0xa1938 associating 1 connmand[5710]: src/service.c:connman_service_lookup_from_network() network 0xa1938 connmand[5710]: src/service.c:__connman_service_ipconfig_indicate_state() service 0x9b078 (wifi_980c82bd8e7d_4775657374_managed_none) state 2 (association) type 1 (IPv4) connmand[5710]: src/service.c:service_indicate_state() service 0x9b078 old idle - new association/idle => association connmand[5710]: src/session.c:service_state_changed() service 0x9b078 state 2 connmand[5710]: src/notifier.c:notify_idle_state() idle 0 connmand[5710]: src/manager.c:idle_state() idle 0 connmand[5710]: src/connection.c:update_order() connmand[5710]: src/connection.c:__connman_connection_update_gateway() default (nil) connmand[5710]: src/service.c:__connman_service_ipconfig_indicate_state() service 0x9b078 (wifi_980c82bd8e7d_4775657374_managed_none) state 2 (association) type 2 (IPv6) connmand[5710]: src/service.c:service_indicate_state() service 0x9b078 old association - new association/association => association connmand[5710]: src/rtnl.c:rtnl_message() buf 0xbe9917b4 len 56 connmand[5710]: src/rtnl.c:rtnl_message() NEWLINK len 56 type 16 flags 0x seq 0 pid 0 connmand[5710]: src/rtnl.c:rtnl_message() buf 0xbe9917b4 len 56 connmand[5710]: src/rtnl.c:rtnl_message() NEWLINK len 56 type 16 flags 0x seq 0 pid 0 connmand[5710]: src/technology.c:scan() technology 0xa4a00 request from :1.200 connmand[5710]: plugins/wifi.c:wifi_scan() device 0x9b258 wifi 0xa5120 hidden ssid (null) connmand[5710]: src/rtnl.c:rtnl_message() buf 0xbe9917b4 len 60 connmand[5710]: src/rtnl.c:rtnl_message() NEWLINK len 60 type 16 flags 0x seq 0 pid 0 connmand[5710]: src/rtnl.c:rtnl_message() buf 0xbe9917b4 len 64 connmand[5710]: src/rtnl.c:rtnl_message() NEWLINK len 64 type 16 flags 0x seq 0 pid 0 connmand[5710]: plugins/wifi.c:connect_callback() network 0xa1938 result 0 connmand[5710]: plugins/wifi.c:interface_state() wifi 0xa1e48 interface state 6 connmand[5710]: plugins/wifi.c:reset_autoscan() connmand[5710]: src/device.c:connman_device_unref_debug() 0x9b258 ref 3 by plugins/wifi.c:231:reset_autoscan() connmand[5710]: src/device.c:connman_device_set_scanning() device 0x9b258 scanning 0 connmand[5710]: src/device.c:free_network() network 0xa1938 connma
Re: [PATCH v1 04/24] session: Add callback to policy create()
Hi,
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> From: Daniel Wagner
>
> Instead returning directly a config when create() is called
> in policy plugin, use a callback function for handing over a valid
> configuration from the plugin to the session core. This prepars
> support for asynchronize create call.
> ---
> include/session.h| 9 +++--
> plugins/session_policy.c | 11 +++
> src/session.c| 29 -
> 3 files changed, 34 insertions(+), 15 deletions(-)
>
> diff --git a/include/session.h b/include/session.h
> index 63e077b..9b6428f 100644
> --- a/include/session.h
> +++ b/include/session.h
> @@ -65,11 +65,16 @@ struct connman_session_config {
> GSList *allowed_bearers;
> };
>
> +typedef void (* connman_session_config_cb) (struct connman_session *session,
> + struct connman_session_config *config,
> + void *user_data);
> +
> struct connman_session_policy {
> const char *name;
> int priority;
> - struct connman_session_config *(*create)(
> - struct connman_session *session);
> + int (*create)(struct connman_session *session,
> + connman_session_config_cb callback,
> + void *user_data);
> void (*destroy)(struct connman_session *session);
> };
>
> diff --git a/plugins/session_policy.c b/plugins/session_policy.c
> index 98d984a..d35d850 100644
> --- a/plugins/session_policy.c
> +++ b/plugins/session_policy.c
> @@ -35,8 +35,9 @@
>
> static GHashTable *config_hash;
>
> -static struct connman_session_config *policy_create(
> - struct connman_session *session)
> +static int policy_create(struct connman_session *session,
> + connman_session_config_cb callback,
> + void *user_data)
> {
> struct connman_session_config *config;
>
> @@ -44,11 +45,13 @@ static struct connman_session_config *policy_create(
>
> config = connman_session_create_default_config();
> if (config == NULL)
> - return NULL;
> + return -ENOMEM;
>
> g_hash_table_replace(config_hash, session, config);
>
> - return config;
> + (*callback)(session, config, user_data);
> +
> + return 0;
> }
Both session and callback need to be checked that they are not NULL.
Return an appropriate error if they are.
> static void policy_destroy(struct connman_session *session)
> diff --git a/src/session.c b/src/session.c
> index e489742..ddc5252 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -236,21 +236,23 @@ static int assign_policy_plugin(struct connman_session
> *session)
> return 0;
> }
>
> -static int create_policy_config(struct connman_session *session)
> +static int create_policy_config(struct connman_session *session,
> + connman_session_config_cb callback,
> + void *user_data)
> {
> struct connman_session_config *config;
>
> - if (session->policy == NULL)
> + if (session->policy == NULL) {
> config = connman_session_create_default_config();
> - else
> - config = (*session->policy->create)(session);
> + if (config == NULL)
> + return -ENOMEM;
>
> - if (config == NULL)
> - return -ENOMEM;
> + session->policy_config = config;
>
> - session->policy_config = config;
> + return 0;
> + }
>
> - return 0;
> + return (*session->policy->create)(session, callback, user_data);
> }
>
> static void destroy_policy_config(struct connman_session *session)
> @@ -1582,6 +1584,15 @@ static const GDBusMethodTable session_methods[] = {
> { },
> };
>
> +static void session_create_cb(struct connman_session *session,
> + struct connman_session_config *config,
> + void *user_data)
> +{
> + DBG("session %p config %p", session, config);
> +
> + session->policy_config = config;
> +}
> +
> int __connman_session_create(DBusMessage *msg)
> {
> const char *owner, *notify_path;
> @@ -1698,7 +1709,7 @@ int __connman_session_create(DBusMessage *msg)
> err = assign_policy_plugin(session);
> if (err < 0)
> goto err;
> - err = create_policy_config(session);
> + err = create_policy_config(session, session_create_cb, NULL);
> if (err < 0)
> goto err;
>
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 04/24] session: Add callback to policy create()
Hi again,
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> From: Daniel Wagner
>
> Instead returning directly a config when create() is called
> in policy plugin, use a callback function for handing over a valid
> configuration from the plugin to the session core. This prepars
> support for asynchronize create call.
> ---
> include/session.h| 9 +++--
> plugins/session_policy.c | 11 +++
> src/session.c| 29 -
> 3 files changed, 34 insertions(+), 15 deletions(-)
>
> diff --git a/include/session.h b/include/session.h
> index 63e077b..9b6428f 100644
> --- a/include/session.h
> +++ b/include/session.h
> @@ -65,11 +65,16 @@ struct connman_session_config {
> GSList *allowed_bearers;
> };
>
> +typedef void (* connman_session_config_cb) (struct connman_session *session,
> + struct connman_session_config *config,
> + void *user_data);
> +
> struct connman_session_policy {
> const char *name;
> int priority;
> - struct connman_session_config *(*create)(
> - struct connman_session *session);
> + int (*create)(struct connman_session *session,
> + connman_session_config_cb callback,
> + void *user_data);
> void (*destroy)(struct connman_session *session);
> };
>
> diff --git a/plugins/session_policy.c b/plugins/session_policy.c
> index 98d984a..d35d850 100644
> --- a/plugins/session_policy.c
> +++ b/plugins/session_policy.c
> @@ -35,8 +35,9 @@
>
> static GHashTable *config_hash;
>
> -static struct connman_session_config *policy_create(
> - struct connman_session *session)
> +static int policy_create(struct connman_session *session,
> + connman_session_config_cb callback,
> + void *user_data)
> {
> struct connman_session_config *config;
>
> @@ -44,11 +45,13 @@ static struct connman_session_config *policy_create(
>
> config = connman_session_create_default_config();
> if (config == NULL)
> - return NULL;
> + return -ENOMEM;
>
> g_hash_table_replace(config_hash, session, config);
>
> - return config;
> + (*callback)(session, config, user_data);
> +
> + return 0;
> }
>
> static void policy_destroy(struct connman_session *session)
> diff --git a/src/session.c b/src/session.c
> index e489742..ddc5252 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -236,21 +236,23 @@ static int assign_policy_plugin(struct connman_session
> *session)
> return 0;
> }
>
> -static int create_policy_config(struct connman_session *session)
> +static int create_policy_config(struct connman_session *session,
> + connman_session_config_cb callback,
> + void *user_data)
> {
> struct connman_session_config *config;
>
> - if (session->policy == NULL)
> + if (session->policy == NULL) {
> config = connman_session_create_default_config();
> - else
> - config = (*session->policy->create)(session);
> + if (config == NULL)
> + return -ENOMEM;
>
> - if (config == NULL)
> - return -ENOMEM;
> + session->policy_config = config;
>
> - session->policy_config = config;
> + return 0;
> + }
>
> - return 0;
> + return (*session->policy->create)(session, callback, user_data);
Here also a check for the create function is needed before calling it.
> }
>
> static void destroy_policy_config(struct connman_session *session)
> @@ -1582,6 +1584,15 @@ static const GDBusMethodTable session_methods[] = {
> { },
> };
>
> +static void session_create_cb(struct connman_session *session,
> + struct connman_session_config *config,
> + void *user_data)
> +{
> + DBG("session %p config %p", session, config);
> +
> + session->policy_config = config;
> +}
> +
> int __connman_session_create(DBusMessage *msg)
> {
> const char *owner, *notify_path;
> @@ -1698,7 +1709,7 @@ int __connman_session_create(DBusMessage *msg)
> err = assign_policy_plugin(session);
> if (err < 0)
> goto err;
> - err = create_policy_config(session);
> + err = create_policy_config(session, session_create_cb, NULL);
> if (err < 0)
> goto err;
>
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH] wifi: Don't set scanning to FALSE during connecting
Hi Arron, Ok I see, but actually your patch is wrong because if autoscan fallback has started, it put scanning to true, and at those place you really need to put scanning to false. Instead, you might try a patch on src/device.c in function mark_network_unavailable(): change line: if (connman_network_get_connected(network) == TRUE) by: if (connman_network_get_connected(network) == TRUE || connman_network_get_connecting(network) == TRUE) can you try that? Tomasz ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 05/24] session: Factor out user settings in __connman_session_create()
Hi,
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> In order to be able to passing the user configuration provided
> through the D-Bus Manager.SessionCreate() call to the callback
> we need to store the configuration into a local data data structure.
> This can then be passed into the callback we introduce in the following
> patches.
> ---
> src/session.c | 50 ++
> 1 file changed, 34 insertions(+), 16 deletions(-)
>
> diff --git a/src/session.c b/src/session.c
> index ddc5252..b1f11de 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -1584,6 +1584,14 @@ static const GDBusMethodTable session_methods[] = {
> { },
> };
>
> +struct user_config {
> + enum connman_session_type type;
> + connman_bool_t type_valid;
> +
> + GSList *allowed_bearers;
> + connman_bool_t allowed_bearers_valid;
> +};
> +
> static void session_create_cb(struct connman_session *session,
> struct connman_session_config *config,
> void *user_data)
> @@ -1600,10 +1608,7 @@ int __connman_session_create(DBusMessage *msg)
> DBusMessageIter iter, array;
> struct connman_session *session = NULL;
> struct session_info *info, *info_last;
> - enum connman_session_type type = CONNMAN_SESSION_TYPE_ANY;
> - GSList *allowed_bearers = NULL;
> - connman_bool_t allowed_bearers_valid = FALSE;
> - connman_bool_t type_valid = FALSE;
> + struct user_config *config = NULL;
> int err;
>
> owner = dbus_message_get_sender(msg);
> @@ -1619,6 +1624,14 @@ int __connman_session_create(DBusMessage *msg)
> goto err;
> }
>
> + config = g_try_new0(struct user_config, 1);
> + if (config == NULL) {
> + err = -ENOMEM;
> + goto err;
> + }
> +
> + config->type = CONNMAN_SESSION_TYPE_ANY;
> +
> dbus_message_iter_init(msg, &iter);
> dbus_message_iter_recurse(&iter, &array);
>
> @@ -1636,11 +1649,11 @@ int __connman_session_create(DBusMessage *msg)
> case DBUS_TYPE_ARRAY:
> if (g_str_equal(key, "AllowedBearers") == TRUE) {
> err = session_parse_allowed_bearers(&value,
> - &allowed_bearers);
> + &config->allowed_bearers);
> if (err < 0)
> goto err;
>
> - allowed_bearers_valid = TRUE;
> + config->allowed_bearers_valid = TRUE;
> } else {
> return -EINVAL;
> }
> @@ -1648,8 +1661,8 @@ int __connman_session_create(DBusMessage *msg)
> case DBUS_TYPE_STRING:
> if (g_str_equal(key, "ConnectionType") == TRUE) {
> dbus_message_iter_get_basic(&value, &val);
> - type = string2type(val);
> - type_valid = TRUE;
> + config->type = string2type(val);
> + config->type_valid = TRUE;
> } else {
> return -EINVAL;
> }
> @@ -1717,18 +1730,18 @@ int __connman_session_create(DBusMessage *msg)
> ecall_session = session;
>
> info->state = CONNMAN_SESSION_STATE_DISCONNECTED;
> - if (type_valid == FALSE)
> - type = CONNMAN_SESSION_TYPE_ANY;
> + if (config->type_valid == FALSE)
> + config->type = CONNMAN_SESSION_TYPE_ANY;
Up there config->type is first set to CONNMAN_SESSION_TYPE_ANY. If there
is a "ConnectionType" set, config->type is set to the given value with
string2type(val) and config->type_valid is set to TRUE. Here it is reset
back to CONNMAN_SESSION_TYPE_ANY if config->type_valid == FALSE...
Same with config->allowed_bearers_valid. I think you can remove both
*_valid variables.
> info->config.type = apply_policy_on_type(
> session->policy_config->type,
> - type);
> + config->type);
> info->config.priority = session->policy_config->priority;
> info->config.roaming_policy = session->policy_config->roaming_policy;
> info->entry = NULL;
>
> - if (allowed_bearers_valid == FALSE) {
> - allowed_bearers = connman_session_allowed_bearers_any();
> - if (allowed_bearers == NULL) {
> + if (config->allowed_bearers_valid == FALSE) {
> + config->allowed_bearers = connman_session_allowed_bearers_any();
> + if (config->allowed_bearers == NULL) {
> err = -ENOMEM;
> goto err;
> }
> @@ -1736,11 +1749,14 @@ int __connman_session_create(DBusMessage *msg)
>
[PATCH 1/3] service: We might know the passphrase for SSID already
We might know the passphrase for a new wifi service if we have
multiple wifi cards and we have successfully connected to known
wifi service at least once. So in this case try to connect to
AP using credentials from already known service.
---
src/service.c | 116 +++---
1 file changed, 111 insertions(+), 5 deletions(-)
diff --git a/src/service.c b/src/service.c
index 7381af3..c479143 100644
--- a/src/service.c
+++ b/src/service.c
@@ -122,6 +122,7 @@ struct connman_service {
connman_bool_t hidden_service;
char *config_file;
char *config_entry;
+ connman_bool_t ignore_connect_retry;
};
static connman_bool_t allow_property_changed(struct connman_service *service);
@@ -5176,22 +5177,49 @@ static int service_indicate_state(struct
connman_service *service)
int __connman_service_indicate_error(struct connman_service *service,
enum connman_service_error error)
{
- DBG("service %p error %d", service, error);
+ connman_bool_t new_connect_failed;
+
+ DBG("service %p error %d -> %d", service, service->error, error);
if (service == NULL)
return -EINVAL;
- set_error(service, error);
+ new_connect_failed = error == CONNMAN_SERVICE_ERROR_CONNECT_FAILED &&
+ connman_service_get_favorite(service) == FALSE &&
+ __connman_service_get_passphrase(service) != NULL;
+
+ if (service->ignore_connect_retry == FALSE &&
+ new_connect_failed == TRUE) {
+ /*
+* We are here if the password we tried was not correct
+* one. So the next __connman_service_connect() will ask it
+* from the user.
+*/
+ DBG("retrying connect...");
- if (service->error == CONNMAN_SERVICE_ERROR_INVALID_KEY)
+ service->ignore_connect_retry = TRUE;
__connman_service_set_passphrase(service, NULL);
+ service->state = CONNMAN_SERVICE_STATE_IDLE;
+ service->error = CONNMAN_SERVICE_ERROR_INVALID_KEY;
+ service->userconnect = TRUE;
- __connman_service_ipconfig_indicate_state(service,
+ __connman_service_connect(service);
+
+ } else {
+ set_error(service, error);
+
+ if (error == CONNMAN_SERVICE_ERROR_INVALID_KEY ||
+ new_connect_failed == TRUE)
+ __connman_service_set_passphrase(service, NULL);
+
+ __connman_service_ipconfig_indicate_state(service,
CONNMAN_SERVICE_STATE_FAILURE,
CONNMAN_IPCONFIG_TYPE_IPV4);
- __connman_service_ipconfig_indicate_state(service,
+ __connman_service_ipconfig_indicate_state(service,
CONNMAN_SERVICE_STATE_FAILURE,
CONNMAN_IPCONFIG_TYPE_IPV6);
+ }
+
return 0;
}
@@ -5538,6 +5566,75 @@ static void prepare_8021x(struct connman_service
*service)
service->phase2);
}
+static
+struct connman_service *check_if_already_known(struct connman_service *service)
+{
+ GSequenceIter *iter;
+ unsigned int service_len;
+ const unsigned char *service_ssid;
+
+ /*
+* The error is set to INVALID_KEY if we have tried the key already.
+*/
+ DBG("ignore %d error %d", service->ignore_connect_retry,
+ service->error);
+
+ if (service->error == CONNMAN_SERVICE_ERROR_INVALID_KEY ||
+ service->ignore_connect_retry == TRUE)
+ return NULL;
+
+ service_ssid = connman_network_get_blob(service->network,
+ "WiFi.SSID", &service_len);
+
+ iter = g_sequence_get_begin_iter(service_list);
+
+ while (g_sequence_iter_is_end(iter) == FALSE) {
+ struct connman_service *other = g_sequence_get(iter);
+ unsigned int other_len;
+ const unsigned char *other_ssid;
+
+ if (other->type != CONNMAN_SERVICE_TYPE_WIFI ||
+ other->type != service->type ||
+ other->favorite != TRUE ||
+ other->ignore != FALSE ||
+ other->immutable != FALSE ||
+ (other->security !=
+ CONNMAN_SERVICE_SECURITY_WEP &&
+ other->security !=
+ CONNMAN_SERVICE_SECURITY_PSK) ||
+ other->security != service->security)
+ goto
[PATCH 0/3] Check if we already know the wifi AP credentials
Hi, if the system has multiple wifi cards and user has connected to access point successfully using one of the cards, then we know the credentials to that AP. So we can reuse this information when connecting to the same AP using different card (=service). So in patch #1 we check during connecting phase if we have same AP already configured in the system. If such AP is found, then we use the credentials of that AP, currently we reuse only credentials if the AP security is WEP or PSK. If the connection with these credentials fail, then we fallback to old behaviour and use agent to ask the passphrase from the user. Patch #2 adds a debug print to show what error we are emitting. This is useful when figuring out if the UI is showing correct error. Patch #3 changes the order when to set the error property. We should not set the error before changing service state because that could overwrite the error too early (done in service_indicate_state() function). So the error property is set only after we have changed the service state. This is important if the system is using both IPv4 and IPv6 as the final state of the system is only known after we have set both sub-states and only after that we are able to determine how the error property is to be set. Cheers, Jukka Jukka Rissanen (3): service: We might know the passphrase for SSID already service: Debug print the error we are sending service: Set error after changing state src/service.c | 118 +++--- 1 file changed, 113 insertions(+), 5 deletions(-) -- 1.7.11.4 ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
[PATCH 3/3] service: Set error after changing state
We should not set the error before state change because
the state change might clear the error in service_indicate_state().
A safe option is set the state after we have changed the state.
---
src/service.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/service.c b/src/service.c
index ebc8371..9316837 100644
--- a/src/service.c
+++ b/src/service.c
@@ -5208,8 +5208,6 @@ int __connman_service_indicate_error(struct
connman_service *service,
__connman_service_connect(service);
} else {
- set_error(service, error);
-
if (error == CONNMAN_SERVICE_ERROR_INVALID_KEY ||
new_connect_failed == TRUE)
__connman_service_set_passphrase(service, NULL);
@@ -5220,6 +5218,8 @@ int __connman_service_indicate_error(struct
connman_service *service,
__connman_service_ipconfig_indicate_state(service,
CONNMAN_SERVICE_STATE_FAILURE,
CONNMAN_IPCONFIG_TYPE_IPV6);
+
+ set_error(service, error);
}
return 0;
--
1.7.11.4
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
[PATCH 2/3] service: Debug print the error we are sending
This is useful when checking whether agent is giving correct error.
---
src/service.c | 2 ++
1 file changed, 2 insertions(+)
diff --git a/src/service.c b/src/service.c
index c479143..ebc8371 100644
--- a/src/service.c
+++ b/src/service.c
@@ -3285,6 +3285,8 @@ static void set_error(struct connman_service *service,
if (allow_property_changed(service) == FALSE)
return;
+ DBG("property error \"%s\"", str);
+
connman_dbus_property_changed_basic(service->path,
CONNMAN_SERVICE_INTERFACE, "Error",
DBUS_TYPE_STRING, &str);
--
1.7.11.4
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 07/24] session: Update sessions on config updates
Hi,
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> From: Daniel Wagner
>
> Give the policy plugin a way to inform the session core that
> some of the config values have changed.
>
> This could be done in a more clever way, e.g. figure out only
> to update the necessary info entries.
>
> It is not expected that there are many updates so let's keep it
> simple for the time beeing.
> ---
> include/session.h | 2 ++
> src/session.c | 37 +
> 2 files changed, 39 insertions(+)
>
> diff --git a/include/session.h b/include/session.h
> index 9b6428f..086c0fc 100644
> --- a/include/session.h
> +++ b/include/session.h
> @@ -81,6 +81,8 @@ struct connman_session_policy {
> int connman_session_policy_register(struct connman_session_policy *config);
> void connman_session_policy_unregister(struct connman_session_policy
> *config);
>
> +void connman_session_config_update(struct connman_session *session);
> +
> GSList *connman_session_allowed_bearers_any(void);
> void connman_session_free_bearers(GSList *bearers);
>
> diff --git a/src/session.c b/src/session.c
> index a5fa801..4778b76 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -1403,6 +1403,43 @@ static void session_changed(struct connman_session
> *session,
> session_notify(session);
> }
>
> +void connman_session_config_update(struct connman_session *session)
> +{
> + struct session_info *info = session->info;
> + GSList *allowed_bearers;
> + int err;
> +
> + DBG("session %p", session);
> +
> + /*
> + * We update all configuration even though only one entry
> + * might have changed. We can still optimize this later.
> + */
> +
> + err = apply_policy_on_bearers(
> + session->policy_config->allowed_bearers,
> + info->config.allowed_bearers,
> + &allowed_bearers);
> + if (err < 0)
> + return;
> + connman_session_free_bearers(info->config.allowed_bearers);
> + info->config.allowed_bearers = allowed_bearers;
> +
> + info->config.type = apply_policy_on_type(
> + session->policy_config->type,
> + info->config.type);
> +
> + info->config.roaming_policy = session->policy_config->roaming_policy;
> +
> + info->config.ecall = session->policy_config->ecall;
> + if (info->config.ecall == TRUE)
> + ecall_session = session;
> +
> + info->config.priority = session->policy_config->priority;
> +
> + session_changed(session, CONNMAN_SESSION_TRIGGER_SETTING);
> +}
> +
> static DBusMessage *connect_session(DBusConnection *conn,
> DBusMessage *msg, void *user_data)
> {
In this patch it starts to become really confusing what an 'info' is. It
would be great if it could be given a more obvious name. Also, it would
help if either 'session->info' were to be used instead of 'info' here,
or that 'session->policy_config' were to be accessed as 'allowed_policy'
or something. Now there is an "imbalance", which makes the source of all
the structures hard to grasp. So that it is absolutely clear which one
is carved in stone, which one is the requested part and where the end
result goes. Just my $0.02 here, let's continue with this patch set
before hastily renaming things.
Cheers,
Patrik
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 00/24] Policy IVI Plugin
Hi, On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote: > From: Daniel Wagner > > Hi, > > The first patch is something I found during writing this code. So > it is kind of unrelated. > > The patches up to #7 are the base work for the IVI policy plugin. I think > we agreed on IRC that this should go in indepenend of the rest. > > From patch #8 on the infrastructure code for the IVI plugin is added. Could you split up this patch set into at least the base work and IVI plugin parts? It becomes very hard to follow-up and apply if all 24 patches will be resent every time... Cheers, Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 04/24] session: Add callback to policy create()
Hi Daniel,
On 17.10.2012 15:42, Daniel Wagner wrote:
From: Daniel Wagner
Instead returning directly a config when create() is called
in policy plugin, use a callback function for handing over a valid
configuration from the plugin to the session core. This prepars
support for asynchronize create call.
---
include/session.h| 9 +++--
plugins/session_policy.c | 11 +++
src/session.c| 29 -
3 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/include/session.h b/include/session.h
index 63e077b..9b6428f 100644
--- a/include/session.h
+++ b/include/session.h
@@ -65,11 +65,16 @@ struct connman_session_config {
GSList *allowed_bearers;
};
+typedef void (* connman_session_config_cb) (struct connman_session *session,
+ struct connman_session_config *config,
+ void *user_data);
+
struct connman_session_policy {
const char *name;
int priority;
- struct connman_session_config *(*create)(
- struct connman_session *session);
+ int (*create)(struct connman_session *session,
+ connman_session_config_cb callback,
+ void *user_data);
void (*destroy)(struct connman_session *session);
};
diff --git a/plugins/session_policy.c b/plugins/session_policy.c
index 98d984a..d35d850 100644
--- a/plugins/session_policy.c
+++ b/plugins/session_policy.c
@@ -35,8 +35,9 @@
static GHashTable *config_hash;
-static struct connman_session_config *policy_create(
- struct connman_session *session)
+static int policy_create(struct connman_session *session,
+ connman_session_config_cb callback,
+ void *user_data)
{
struct connman_session_config *config;
@@ -44,11 +45,13 @@ static struct connman_session_config *policy_create(
config = connman_session_create_default_config();
if (config == NULL)
- return NULL;
+ return -ENOMEM;
g_hash_table_replace(config_hash, session, config);
- return config;
+ (*callback)(session, config, user_data);
+
+ return 0;
}
static void policy_destroy(struct connman_session *session)
diff --git a/src/session.c b/src/session.c
index e489742..ddc5252 100644
--- a/src/session.c
+++ b/src/session.c
@@ -236,21 +236,23 @@ static int assign_policy_plugin(struct connman_session
*session)
After applying the patch I noticed that the assign_policy_plugin() could
be refactored a bit.
for (list = policy_list; list != NULL; list = list->next) {
policy = list->data;
session->policy = policy;
break;
}
The for loop is useless here as you could take the first entry from the
policy_list and use that.
return 0;
}
-static int create_policy_config(struct connman_session *session)
+static int create_policy_config(struct connman_session *session,
+ connman_session_config_cb callback,
+ void *user_data)
{
struct connman_session_config *config;
- if (session->policy == NULL)
+ if (session->policy == NULL) {
config = connman_session_create_default_config();
- else
- config = (*session->policy->create)(session);
+ if (config == NULL)
+ return -ENOMEM;
- if (config == NULL)
- return -ENOMEM;
+ session->policy_config = config;
- session->policy_config = config;
+ return 0;
+ }
- return 0;
+ return (*session->policy->create)(session, callback, user_data);
}
static void destroy_policy_config(struct connman_session *session)
@@ -1582,6 +1584,15 @@ static const GDBusMethodTable session_methods[] = {
{ },
};
+static void session_create_cb(struct connman_session *session,
+ struct connman_session_config *config,
+ void *user_data)
+{
+ DBG("session %p config %p", session, config);
+
+ session->policy_config = config;
+}
+
int __connman_session_create(DBusMessage *msg)
{
const char *owner, *notify_path;
@@ -1698,7 +1709,7 @@ int __connman_session_create(DBusMessage *msg)
err = assign_policy_plugin(session);
if (err < 0)
goto err;
- err = create_policy_config(session);
+ err = create_policy_config(session, session_create_cb, NULL);
if (err < 0)
goto err;
Cheers,
Jukka
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 00/24] Policy IVI Plugin
Hi Patrik, On 19.10.2012 12:52, Patrik Flykt wrote: Hi, On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote: From: Daniel Wagner Hi, The first patch is something I found during writing this code. So it is kind of unrelated. The patches up to #7 are the base work for the IVI policy plugin. I think we agreed on IRC that this should go in indepenend of the rest. From patch #8 on the infrastructure code for the IVI plugin is added. Could you split up this patch set into at least the base work and IVI plugin parts? It becomes very hard to follow-up and apply if all 24 patches will be resent every time... Sure, I collected them so that I have a complete set when working on this feature. Let's focus on the first part then (up to patch #7). cheers, daniel ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 04/24] session: Add callback to policy create()
On 19.10.2012 12:11, Patrik Flykt wrote:
Hi again,
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
From: Daniel Wagner
Instead returning directly a config when create() is called
in policy plugin, use a callback function for handing over a valid
configuration from the plugin to the session core. This prepars
support for asynchronize create call.
---
include/session.h| 9 +++--
plugins/session_policy.c | 11 +++
src/session.c| 29 -
3 files changed, 34 insertions(+), 15 deletions(-)
diff --git a/include/session.h b/include/session.h
index 63e077b..9b6428f 100644
--- a/include/session.h
+++ b/include/session.h
@@ -65,11 +65,16 @@ struct connman_session_config {
GSList *allowed_bearers;
};
+typedef void (* connman_session_config_cb) (struct connman_session *session,
+ struct connman_session_config *config,
+ void *user_data);
+
struct connman_session_policy {
const char *name;
int priority;
- struct connman_session_config *(*create)(
- struct connman_session *session);
+ int (*create)(struct connman_session *session,
+ connman_session_config_cb callback,
+ void *user_data);
void (*destroy)(struct connman_session *session);
};
diff --git a/plugins/session_policy.c b/plugins/session_policy.c
index 98d984a..d35d850 100644
--- a/plugins/session_policy.c
+++ b/plugins/session_policy.c
@@ -35,8 +35,9 @@
static GHashTable *config_hash;
-static struct connman_session_config *policy_create(
- struct connman_session *session)
+static int policy_create(struct connman_session *session,
+ connman_session_config_cb callback,
+ void *user_data)
{
struct connman_session_config *config;
@@ -44,11 +45,13 @@ static struct connman_session_config *policy_create(
config = connman_session_create_default_config();
if (config == NULL)
- return NULL;
+ return -ENOMEM;
g_hash_table_replace(config_hash, session, config);
- return config;
+ (*callback)(session, config, user_data);
+
+ return 0;
}
static void policy_destroy(struct connman_session *session)
diff --git a/src/session.c b/src/session.c
index e489742..ddc5252 100644
--- a/src/session.c
+++ b/src/session.c
@@ -236,21 +236,23 @@ static int assign_policy_plugin(struct connman_session
*session)
return 0;
}
-static int create_policy_config(struct connman_session *session)
+static int create_policy_config(struct connman_session *session,
+ connman_session_config_cb callback,
+ void *user_data)
{
struct connman_session_config *config;
- if (session->policy == NULL)
+ if (session->policy == NULL) {
config = connman_session_create_default_config();
- else
- config = (*session->policy->create)(session);
+ if (config == NULL)
+ return -ENOMEM;
- if (config == NULL)
- return -ENOMEM;
+ session->policy_config = config;
- session->policy_config = config;
+ return 0;
+ }
- return 0;
+ return (*session->policy->create)(session, callback, user_data);
Here also a check for the create function is needed before calling it.
Good catch. Will fix it (also the other places)
cheers,
daniel
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 07/24] session: Update sessions on config updates
On 19.10.2012 12:47, Patrik Flykt wrote:
Hi,
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
From: Daniel Wagner
Give the policy plugin a way to inform the session core that
some of the config values have changed.
This could be done in a more clever way, e.g. figure out only
to update the necessary info entries.
It is not expected that there are many updates so let's keep it
simple for the time beeing.
---
include/session.h | 2 ++
src/session.c | 37 +
2 files changed, 39 insertions(+)
diff --git a/include/session.h b/include/session.h
index 9b6428f..086c0fc 100644
--- a/include/session.h
+++ b/include/session.h
@@ -81,6 +81,8 @@ struct connman_session_policy {
int connman_session_policy_register(struct connman_session_policy *config);
void connman_session_policy_unregister(struct connman_session_policy *config);
+void connman_session_config_update(struct connman_session *session);
+
GSList *connman_session_allowed_bearers_any(void);
void connman_session_free_bearers(GSList *bearers);
diff --git a/src/session.c b/src/session.c
index a5fa801..4778b76 100644
--- a/src/session.c
+++ b/src/session.c
@@ -1403,6 +1403,43 @@ static void session_changed(struct connman_session
*session,
session_notify(session);
}
+void connman_session_config_update(struct connman_session *session)
+{
+ struct session_info *info = session->info;
+ GSList *allowed_bearers;
+ int err;
+
+ DBG("session %p", session);
+
+ /*
+* We update all configuration even though only one entry
+* might have changed. We can still optimize this later.
+*/
+
+ err = apply_policy_on_bearers(
+ session->policy_config->allowed_bearers,
+ info->config.allowed_bearers,
+ &allowed_bearers);
+ if (err < 0)
+ return;
+ connman_session_free_bearers(info->config.allowed_bearers);
+ info->config.allowed_bearers = allowed_bearers;
+
+ info->config.type = apply_policy_on_type(
+ session->policy_config->type,
+ info->config.type);
+
+ info->config.roaming_policy = session->policy_config->roaming_policy;
+
+ info->config.ecall = session->policy_config->ecall;
+ if (info->config.ecall == TRUE)
+ ecall_session = session;
+
+ info->config.priority = session->policy_config->priority;
+
+ session_changed(session, CONNMAN_SESSION_TRIGGER_SETTING);
+}
+
static DBusMessage *connect_session(DBusConnection *conn,
DBusMessage *msg, void *user_data)
{
In this patch it starts to become really confusing what an 'info' is.
I completely agree.
It would be great if it could be given a more obvious name.
If you have good ideas I am more than happy to fix it.
Also, it would help if either 'session->info' were to be used instead of 'info'
here,
The reasons why I am using info instead of session->info is one to
follow the code pattern in the rest of the file and also to avoid having
long lines. I agree the whole info and last_info is kind
of confusing. Maybe we could find a more elegant solution to this
code pattern.
> or that 'session->policy_config' were to be accessed as
> 'allowed_policy' or something.
allowed_policy sounds kind of wrong. First I had only policy or config
and it was kind of hard to follow the code. But again if you have good
ideas bring them forward (also bad ones are okay, then I can start
flaming them :P) /me runs away
Now there is an "imbalance", which makes the source of all
the structures hard to grasp. So that it is absolutely clear which one
is carved in stone, which one is the requested part and where the end
result goes. Just my $0.02 here, let's continue with this patch set
before hastily renaming things.
Okay, let's get the first patches right. We need to go over the complete
file in case we want to use a different pattern.
cheers
daniel
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 04/24] session: Add callback to policy create()
Hi Jukka,
On 19.10.2012 12:55, Jukka Rissanen wrote:
After applying the patch I noticed that the assign_policy_plugin() could
be refactored a bit.
for (list = policy_list; list != NULL; list = list->next) {
policy = list->data;
session->policy = policy;
break;
}
The for loop is useless here as you could take the first entry from the
policy_list and use that.
Good idea!
cheers,
daniel
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 04/24] session: Add callback to policy create()
On Fri, Oct 19, 2012 at 01:11:36PM +0300, Patrik Flykt wrote: > > - return 0; > > + return (*session->policy->create)(session, callback, user_data); > > Here also a check for the create function is needed before calling it. I think I am going to add this test to the registration function. What do you think on this? ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 09/24] storage: Move generic inotify into storage.c
Hi Daniel,
this looks nice but there is a small issue when we get the connman-vpn
integrated into connman. The connman-vpnd is linking storage.c as it
needs some functions from there but it does not need config.c. If we
could keep the notify stuff in config, then it would mean smaller
storage and that would be nice for new vpn stuff.
Cheers,
Jukka
On 17.10.2012 15:42, Daniel Wagner wrote:
From: Daniel Wagner
---
include/storage.h | 8 +++
src/config.c | 122 +-
src/connman.h | 5 ++
src/main.c| 2 +
src/storage.c | 155 ++
5 files changed, 172 insertions(+), 120 deletions(-)
diff --git a/include/storage.h b/include/storage.h
index 4c23a14..4582390 100644
--- a/include/storage.h
+++ b/include/storage.h
@@ -28,9 +28,17 @@
extern "C" {
#endif
+struct inotify_event;
+
+typedef void (* storage_notify_cb) (struct inotify_event *event,
+ const char *ident);
+
gchar **connman_storage_get_services();
GKeyFile *connman_storage_load_service(const char *service_id);
+int connman_storage_notify_register(storage_notify_cb callback);
+void connman_storage_notify_unregister(storage_notify_cb callback);
+
#ifdef __cplusplus
}
#endif
diff --git a/src/config.c b/src/config.c
index 16fed8d..d70af8f 100644
--- a/src/config.c
+++ b/src/config.c
@@ -66,10 +66,6 @@ struct connman_config {
static GHashTable *config_table = NULL;
static GSList *protected_services = NULL;
-static int inotify_wd = -1;
-
-static GIOChannel *inotify_channel = NULL;
-static uint inotify_watch = 0;
static connman_bool_t cleanup = FALSE;
#define INTERNAL_CONFIG_PREFIX "__internal"
@@ -613,120 +609,6 @@ static void config_notify_handler(struct inotify_event
*event,
g_hash_table_remove(config_table, ident);
}
-static gboolean inotify_data(GIOChannel *channel, GIOCondition cond,
- gpointer user_data)
-{
- char buffer[256];
- char *next_event;
- gsize bytes_read;
- GIOStatus status;
-
- if (cond & (G_IO_NVAL | G_IO_ERR | G_IO_HUP)) {
- inotify_watch = 0;
- return FALSE;
- }
-
- status = g_io_channel_read_chars(channel, buffer,
- sizeof(buffer) -1, &bytes_read, NULL);
-
- switch (status) {
- case G_IO_STATUS_NORMAL:
- break;
- case G_IO_STATUS_AGAIN:
- return TRUE;
- default:
- connman_error("Reading from inotify channel failed");
- inotify_watch = 0;
- return FALSE;
- }
-
- next_event = buffer;
-
- while (bytes_read > 0) {
- struct inotify_event *event;
- gchar *ident;
- gsize len;
-
- event = (struct inotify_event *) next_event;
- if (event->len)
- ident = next_event + sizeof(struct inotify_event);
- else
- ident = NULL;
-
- len = sizeof(struct inotify_event) + event->len;
-
- /* check if inotify_event block fit */
- if (len > bytes_read)
- break;
-
- next_event += len;
- bytes_read -= len;
-
- config_notify_handler(event, ident);
- }
-
- return TRUE;
-}
-
-static int create_watch(void)
-{
- int fd;
-
- fd = inotify_init();
- if (fd < 0)
- return -EIO;
-
- inotify_wd = inotify_add_watch(fd, STORAGEDIR,
- IN_MODIFY | IN_CREATE | IN_DELETE);
- if (inotify_wd < 0) {
- connman_error("Creation of STORAGEDIR watch failed");
- close(fd);
- return -EIO;
- }
-
- inotify_channel = g_io_channel_unix_new(fd);
- if (inotify_channel == NULL) {
- connman_error("Creation of inotify channel failed");
- inotify_rm_watch(fd, inotify_wd);
- inotify_wd = 0;
-
- close(fd);
- return -EIO;
- }
-
- g_io_channel_set_close_on_unref(inotify_channel, TRUE);
- g_io_channel_set_encoding(inotify_channel, NULL, NULL);
- g_io_channel_set_buffered(inotify_channel, FALSE);
-
- inotify_watch = g_io_add_watch(inotify_channel,
- G_IO_IN | G_IO_HUP | G_IO_NVAL | G_IO_ERR,
- inotify_data, NULL);
-
- return 0;
-}
-
-static void remove_watch(void)
-{
- int fd;
-
- if (inotify_channel == NULL)
- return;
-
- if (inotify_watch > 0) {
- g_source_remove(inotify_watch);
- inotify_watch = 0;
- }
-
- fd = g_io_channel_unix_get_fd(inotify_channel);
-
- if (inotify_wd >= 0) {
-
Re: [PATCH v1 11/24] session: Add connman_session_get_owner()
Hi Daniel,
On 17.10.2012 15:42, Daniel Wagner wrote:
From: Daniel Wagner
---
include/session.h | 3 ++-
src/session.c | 5 +
2 files changed, 7 insertions(+), 1 deletion(-)
diff --git a/include/session.h b/include/session.h
index 086c0fc..afad9fb 100644
--- a/include/session.h
+++ b/include/session.h
@@ -85,9 +85,10 @@ void connman_session_config_update(struct connman_session
*session);
GSList *connman_session_allowed_bearers_any(void);
void connman_session_free_bearers(GSList *bearers);
-
This change is not related to the rest of the patch. Is this change
really needed at all?
struct connman_session_config *connman_session_create_default_config(void);
+const char *connman_session_get_owner(struct connman_session *session);
+
#ifdef __cplusplus
}
#endif
diff --git a/src/session.c b/src/session.c
index 4778b76..b39dd0a 100644
--- a/src/session.c
+++ b/src/session.c
@@ -529,6 +529,11 @@ GSList *connman_session_allowed_bearers_any(void)
return list;
}
+const char *connman_session_get_owner(struct connman_session *session)
+{
+ return session->owner;
+}
+
static void append_allowed_bearers(DBusMessageIter *iter, void *user_data)
{
struct session_info *info = user_data;
Cheers,
Jukka
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 13/24] session_policy_ivi: Add D-Bus connection
Hi Daniel,
the dbus stuff is quite mandatory in this plugin anyway so perhaps this
could be merged with the previous patch.
On 17.10.2012 15:42, Daniel Wagner wrote:
From: Daniel Wagner
---
plugins/session_policy_ivi.c | 18 +-
1 file changed, 17 insertions(+), 1 deletion(-)
diff --git a/plugins/session_policy_ivi.c b/plugins/session_policy_ivi.c
index c11067e..2c30b05 100644
--- a/plugins/session_policy_ivi.c
+++ b/plugins/session_policy_ivi.c
@@ -27,10 +27,15 @@
#include
+#include
+
#define CONNMAN_API_SUBJECT_TO_CHANGE
#include
#include
#include
+#include
+
+static DBusConnection *connection;
static int policy_ivi_create(struct connman_session *session,
connman_session_config_cb callback,
@@ -57,16 +62,27 @@ static int session_policy_ivi_init(void)
{
int err;
+ connection = connman_dbus_get_connection();
+ if (connection == NULL)
+ return -EIO;
+
err = connman_session_policy_register(&session_policy_ivi);
if (err < 0)
- return err;
+ goto err;
return 0;
+
+err:
+ dbus_connection_unref(connection);
+
+ return err;
}
static void session_policy_ivi_exit(void)
{
connman_session_policy_unregister(&session_policy_ivi);
+
+ dbus_connection_unref(connection);
}
CONNMAN_PLUGIN_DEFINE(session_policy_ivi,
Cheers,
Jukka
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 09/24] storage: Move generic inotify into storage.c
Hi Jukka, On 19.10.2012 13:50, Jukka Rissanen wrote: Hi Daniel, this looks nice but there is a small issue when we get the connman-vpn integrated into connman. The connman-vpnd is linking storage.c as it needs some functions from there but it does not need config.c. If we could keep the notify stuff in config, then it would mean smaller storage and that would be nice for new vpn stuff. Is your concern only about the size of storage? Shouldn't the linker through out the inotify stuff if you don't use it? I found it rather confusing to have the inotify part in the config.c file. Another way would to move the inotify part into its own file which I would be nice BTW. cheers, daniel ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 13/24] session_policy_ivi: Add D-Bus connection
Hi Jukka, On 19.10.2012 13:57, Jukka Rissanen wrote: Hi Daniel, the dbus stuff is quite mandatory in this plugin anyway so perhaps this could be merged with the previous patch. Yeah, a too small step. I thought it helps reviewing. Let me squash it to the previous patch. cheers, daniel ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 09/24] storage: Move generic inotify into storage.c
On 19.10.2012 14:58, Daniel Wagner wrote: Hi Jukka, On 19.10.2012 13:50, Jukka Rissanen wrote: Hi Daniel, this looks nice but there is a small issue when we get the connman-vpn integrated into connman. The connman-vpnd is linking storage.c as it needs some functions from there but it does not need config.c. If we could keep the notify stuff in config, then it would mean smaller storage and that would be nice for new vpn stuff. Is your concern only about the size of storage? Shouldn't the linker through out the inotify stuff if you don't use it? I found it rather confusing to have the inotify part in the config.c file. Another way would to move the inotify part into its own file which I would be nice BTW. True. It was just a thought, I am also fine with new funcs in storage.c or some new file. Cheers, Jukka ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 09/24] storage: Move generic inotify into storage.c
On 19.10.2012 14:13, Jukka Rissanen wrote: On 19.10.2012 14:58, Daniel Wagner wrote: Hi Jukka, On 19.10.2012 13:50, Jukka Rissanen wrote: Hi Daniel, this looks nice but there is a small issue when we get the connman-vpn integrated into connman. The connman-vpnd is linking storage.c as it needs some functions from there but it does not need config.c. If we could keep the notify stuff in config, then it would mean smaller storage and that would be nice for new vpn stuff. Is your concern only about the size of storage? Shouldn't the linker through out the inotify stuff if you don't use it? I found it rather confusing to have the inotify part in the config.c file. Another way would to move the inotify part into its own file which I would be nice BTW. True. It was just a thought, I am also fine with new funcs in storage.c or some new file. Having a new file for inotify stuff could help to split the core into smaller pieces which helps readability. ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH 0/3] Check if we already know the wifi AP credentials
Hi Jukka, > if the system has multiple wifi cards and user has connected to > access point successfully using one of the cards, then we know > the credentials to that AP. So we can reuse this information > when connecting to the same AP using different card (=service). so explain to me why this is actually a good idea. Regards Marcel ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 04/24] session: Add callback to policy create()
On fre, 2012-10-19 at 13:47 +0200, Daniel Wagner wrote: > On Fri, Oct 19, 2012 at 01:11:36PM +0300, Patrik Flykt wrote: > > > - return 0; > > > + return (*session->policy->create)(session, callback, user_data); > > > > Here also a check for the create function is needed before calling it. > > I think I am going to add this test to the registration function. What do you > think on this? Yep, always check that pointers are set before calling them. Cheers, Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 09/24] storage: Move generic inotify into storage.c
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote: > From: Daniel Wagner > > --- > include/storage.h | 8 +++ > src/config.c | 122 +- > src/connman.h | 5 ++ > src/main.c| 2 + > src/storage.c | 155 > ++ > 5 files changed, 172 insertions(+), 120 deletions(-) What do we gain by moving around stuff with this patch? Cheers, Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 10/24] gdbus: Add GetConnectionSELinuxSecurityContext
Hi,
gdbus/ should be kept in sync between Bluez, ConnMan, oFono and neard.
Any other code should be added elsewhere.
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> From: Daniel Wagner
>
> ---
> Makefile.am | 2 +-
> gdbus/gdbus.h | 9 +++
> gdbus/selinux.c | 167
>
> 3 files changed, 177 insertions(+), 1 deletion(-)
> create mode 100644 gdbus/selinux.c
>
> diff --git a/Makefile.am b/Makefile.am
> index b845d6e..dbcfd86 100644
> --- a/Makefile.am
> +++ b/Makefile.am
> @@ -23,7 +23,7 @@ local_headers = $(foreach file,$(include_HEADERS)
> $(nodist_include_HEADERS) \
>
>
> gdbus_sources = gdbus/gdbus.h gdbus/mainloop.c gdbus/watch.c \
> - gdbus/object.c gdbus/polkit.c
> + gdbus/object.c gdbus/polkit.c gdbus/selinux.c
>
> gdhcp_sources = gdhcp/gdhcp.h gdhcp/common.h gdhcp/common.c gdhcp/client.c \
> gdhcp/server.c gdhcp/ipv4ll.h gdhcp/ipv4ll.c gdhcp/unaligned.h
> diff --git a/gdbus/gdbus.h b/gdbus/gdbus.h
> index 0a8a27c..c7a7e6f 100644
> --- a/gdbus/gdbus.h
> +++ b/gdbus/gdbus.h
> @@ -62,6 +62,10 @@ typedef void (* GDBusSecurityFunction) (DBusConnection
> *connection,
> gboolean interaction,
> GDBusPendingReply pending);
>
> +typedef void (* GDBusSELinuxFunction) (unsigned char *context,
> + unsigned int size,
> + void *user_data);
> +
> typedef enum {
> G_DBUS_METHOD_FLAG_DEPRECATED = (1 << 0),
> G_DBUS_METHOD_FLAG_NOREPLY= (1 << 1),
> @@ -160,6 +164,11 @@ typedef struct {
> .args = _args, \
> .flags = G_DBUS_SIGNAL_FLAG_DEPRECATED
>
> +gboolean g_dbus_selinux_get_context(DBusConnection *connection,
> + const char *service,
> + GDBusSELinuxFunction func,
> + void *user_data);
> +
> gboolean g_dbus_register_interface(DBusConnection *connection,
> const char *path, const char *name,
> const GDBusMethodTable *methods,
> diff --git a/gdbus/selinux.c b/gdbus/selinux.c
> new file mode 100644
> index 000..f7d932f
> --- /dev/null
> +++ b/gdbus/selinux.c
> @@ -0,0 +1,167 @@
> +/*
> + *
> + * D-Bus helper library
> + *
> + * Copyright (C) 2012 BMW Car IT GmbH. All rights reserved.
> + *
> + *
> + * This program is free software; you can redistribute it and/or modify
> + * it under the terms of the GNU General Public License as published by
> + * the Free Software Foundation; either version 2 of the License, or
> + * (at your option) any later version.
> + *
> + * This program is distributed in the hope that it will be useful,
> + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
> + * GNU General Public License for more details.
> + *
> + * You should have received a copy of the GNU General Public License
> + * along with this program; if not, write to the Free Software
> + * Foundation, Inc., 51 Franklin St, Fifth Floor, Boston, MA 02110-1301
> USA
> + *
> + */
> +
> +#ifdef HAVE_CONFIG_H
> +#include
> +#endif
> +
> +#include
> +
> +#include
> +
> +#include
> +
> +#include "gdbus.h"
> +
> +#define info(fmt...)
> +#define error(fmt...)
> +#define debug(fmt...)
> +
> +struct selinux_data {
> + GDBusSELinuxFunction func;
> + void *user_data;
> +};
> +
> +static int parse_context(DBusMessage *msg, unsigned char **context,
> + unsigned int *size)
> +{
> + DBusMessageIter iter, array;
> + unsigned char *ctx;
> + int i = 0;
> +
> + dbus_message_iter_init(msg, &iter);
> + dbus_message_iter_recurse(&iter, &array);
> + while (dbus_message_iter_get_arg_type(&array) == DBUS_TYPE_BYTE) {
> + i++;
> +
> + dbus_message_iter_next(&array);
> + }
> +
> + *size = i;
> + if (i == 0)
> + return 0;
> +
> + ctx = g_try_malloc((gsize) *size);
> + if (ctx == NULL)
> + return -ENOMEM;
> +
> + i = 0;
> + dbus_message_iter_init(msg, &iter);
> + dbus_message_iter_recurse(&iter, &array);
> + while (dbus_message_iter_get_arg_type(&array) == DBUS_TYPE_BYTE) {
> + dbus_message_iter_get_basic(&array, &ctx[i]);
> +
What are we fetching here, individual bytes or something else? Should
something be strdup'ed if it's not bytes?
> + i++;
> + dbus_message_iter_next(&array);
> + }
> +
> + *context = ctx;
> +
> + return 0;
> +}
> +
Perhaps the parsed char ** could be returned here? Also, is ctx long
enough to end in a NULL?
> +static void selinux_get_context_reply(DBusPendingCall *call, void *user_data)
> +{
> +
Re: [PATCH v1 12/24] session_policy_ivi: Add policy plugin for IVI
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote: > From: Daniel Wagner This and the following patches could make a set of their own, no? Cheers, Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 13/24] session_policy_ivi: Add D-Bus connection
On fre, 2012-10-19 at 14:00 +0200, Daniel Wagner wrote: > Let me squash it to the previous patch. +1 from me. Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 14/24] session_policy_ivi: Get SELinux context of session owner
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> From: Daniel Wagner
>
> ---
> plugins/session_policy_ivi.c | 50
> +++-
> 1 file changed, 49 insertions(+), 1 deletion(-)
>
> diff --git a/plugins/session_policy_ivi.c b/plugins/session_policy_ivi.c
> index 2c30b05..8a7503a 100644
> --- a/plugins/session_policy_ivi.c
> +++ b/plugins/session_policy_ivi.c
> @@ -37,13 +37,61 @@
>
> static DBusConnection *connection;
>
> +struct create_data {
> + struct connman_session *session;
> + connman_session_config_cb callback;
> + void *user_data;
> +};
> +
> +static void selinux_context_reply(unsigned char *context, unsigned int size,
> + void *user_data)
> +{
> + struct create_data *data = user_data;
> + char *str;
> +
> + DBG("session %p", data->session);
> +
> + str = g_strndup((const gchar*)context, (gsize) size);
> + if (str == NULL)
> + goto out;
> +
> + DBG("SELinux context %s", str);
> +
> +out:
> + (*data->callback)(data->session, NULL, data->user_data);
> +
data->callback should be verified that it's not NULL, perhaps already in
policy_ivi_create before attempting the method call.
> + g_free(data);
> + g_free(str);
> +}
> +
> static int policy_ivi_create(struct connman_session *session,
> connman_session_config_cb callback,
> void *user_data)
> {
> + struct create_data *data;
> + const char *owner;
> +
> DBG("session %p", session);
>
> - return -ENOMEM;
> + data = g_try_new0(struct create_data, 1);
> + if (data == NULL)
> + return -ENOMEM;
> +
> + data->session = session;
> + data->callback = callback;
> + data->user_data = user_data;
> +
> + owner = connman_session_get_owner(session);
> +
> + if (g_dbus_selinux_get_context(connection, owner,
> + selinux_context_reply,
> + data) == FALSE) {
> + connman_error("Could not get SELinux context");
> + g_free(data);
> + return -ENXIO;
> + }
> +
> + return 0;
> }
>
> static void policy_ivi_destroy(struct connman_session *session)
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 06/24] session: Register session after policy plugin return config
Hi,
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> Move the configuration part of __connman_session_create() into
> session_create_cb(). With this change the policy plugin is able
> to do async work to retrieve a configuration.
> ---
> src/session.c | 190
> --
> 1 file changed, 106 insertions(+), 84 deletions(-)
>
> diff --git a/src/session.c b/src/session.c
> index b1f11de..a5fa801 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -882,6 +882,9 @@ static gint sort_services(gconstpointer a, gconstpointer
> b, gpointer user_data)
>
> static void free_session(struct connman_session *session)
> {
> + if (session == NULL)
> + return;
> +
> destroy_policy_config(session);
> connman_session_free_bearers(session->info->config.allowed_bearers);
> g_free(session->owner);
> @@ -1585,6 +1588,8 @@ static const GDBusMethodTable session_methods[] = {
> };
>
> struct user_config {
> + DBusMessage *pending;
> +
> enum connman_session_type type;
> connman_bool_t type_valid;
>
> @@ -1596,9 +1601,94 @@ static void session_create_cb(struct connman_session
> *session,
> struct connman_session_config *config,
> void *user_data)
> {
> + DBusMessage *reply;
> + struct user_config *user_config = user_data;
> + struct session_info *info, *info_last;
> + int err = 0;
> +
> DBG("session %p config %p", session, config);
>
> + if (config == NULL) {
> + err = -ENOMEM;
> + goto out;
> + }
> +
In plugin/session_policy_ivi.c in patch 14/24 a NULL config comes also
from no SELinux context, which probably is a security context violation
and should not be reported with -ENOMEM.
Should there be a better/explicit error status for the session create
callback type in general?
> session->policy_config = config;
> +
> + info = session->info;
> + info_last = session->info_last;
> +
> + if (session->policy_config->ecall == TRUE)
> + ecall_session = session;
> +
> + info->state = CONNMAN_SESSION_STATE_DISCONNECTED;
> + if (user_config->type_valid == FALSE)
> + user_config->type = CONNMAN_SESSION_TYPE_ANY;
> + info->config.type = apply_policy_on_type(
> + session->policy_config->type,
> + user_config->type);
> + info->config.priority = session->policy_config->priority;
> + info->config.roaming_policy = session->policy_config->roaming_policy;
> + info->entry = NULL;
> +
> + if (user_config->allowed_bearers_valid == FALSE) {
> + user_config->allowed_bearers =
> + connman_session_allowed_bearers_any();
> + if (user_config->allowed_bearers == NULL) {
> + err = -ENOMEM;
> + goto out;
> + }
> + }
> +
> + err = apply_policy_on_bearers(
> + session->policy_config->allowed_bearers,
> + config->allowed_bearers,
> + &info->config.allowed_bearers);
> + if (err < 0)
> + goto out;
> +
> + g_hash_table_replace(session_hash, session->session_path, session);
> +
> + DBG("add %s", session->session_path);
> +
> + if (g_dbus_register_interface(connection, session->session_path,
> + CONNMAN_SESSION_INTERFACE,
> + session_methods, NULL,
> + NULL, session, NULL) == FALSE) {
> + connman_error("Failed to register %s", session->session_path);
> + g_hash_table_remove(session_hash, session->session_path);
> + err = -EINVAL;
> + goto out;
> + }
> +
> + reply = g_dbus_create_reply(user_config->pending,
> + DBUS_TYPE_OBJECT_PATH, &session->session_path,
> + DBUS_TYPE_INVALID);
> + g_dbus_send_message(connection, reply);
> +
> + populate_service_list(session);
> +
> + info_last->state = info->state;
> + info_last->config.priority = info->config.priority;
> + info_last->config.roaming_policy = info->config.roaming_policy;
> + info_last->entry = info->entry;
> + info_last->config.allowed_bearers = info->config.allowed_bearers;
> +
> + session->append_all = TRUE;
> +
> + session_changed(session, CONNMAN_SESSION_TRIGGER_SETTING);
> +
> +out:
> + if (err < 0) {
> + __connman_error_failed(user_config->pending, err);
> + free_session(session);
> + }
> +
> + dbus_message_unref(user_config->pending);
> +
> + if (user_config != NULL)
> + connman_session_free_bearers(user_config->allowed_bearers);
> + g_free(user_config);
> }
>
> int __connman_session_create(DBusMessage *msg)
> @@ -1607,7 +1697,6 @@ int __con
Re: [PATCH v1 15/24] session_policy_ivi: Create session config
On ons, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote: > From: Daniel Wagner > > Instead using glib's cleanup hooks for the hash table, we implement > it ourself. The reason for doing this is that we will use the > policy_data structure via a second hash table. So there is no > direct ownership between session_hash and policy_data after we add the > second hash table. I only see one hash table in this patch and in the plugin so far. Or then I'm missing something. Please fix up the commit message to reflect the patch. Cheers, Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 16/24] session_policy_ivi: Factor out config creation
Hi, This one could have gone in by default so that this refactoring patch would not be needed. Of course with at commit message stating that new_config() is to be used in a later patch in another place. Cheers, Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
Re: [PATCH v1 17/24] session_policy_ivi: Read policy config from file system
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> From: Daniel Wagner
>
> ---
> plugins/session_policy_ivi.c | 116
> +++
> 1 file changed, 116 insertions(+)
>
> diff --git a/plugins/session_policy_ivi.c b/plugins/session_policy_ivi.c
> index b2e58ee..1040040 100644
> --- a/plugins/session_policy_ivi.c
> +++ b/plugins/session_policy_ivi.c
> @@ -24,6 +24,7 @@
> #endif
>
> #include
> +#include
>
> #include
>
> @@ -38,6 +39,7 @@
> static DBusConnection *connection;
>
> static GHashTable *session_hash;
> +static GHashTable *ident_hash;
Please move ident_hash to the next patch where it belongs. This patch is
just concerned with reading files. It's not at all clear why the
ident_hash exists when reading this patch.
> struct create_data {
> struct connman_session *session;
> @@ -46,6 +48,7 @@ struct create_data {
> };
>
> struct policy_data {
> + char *ident;
> struct connman_session *session;
>
> struct connman_session_config *config;
> @@ -59,6 +62,7 @@ static void cleanup_policy(struct policy_data *policy)
> if (policy->config != NULL)
> connman_session_free_bearers(policy->config->allowed_bearers);
>
> + g_free(policy->ident);
> g_free(policy->config);
> g_free(policy);
> }
> @@ -174,6 +178,101 @@ static struct connman_session_policy session_policy_ivi
> = {
> .destroy = policy_ivi_destroy,
> };
>
> +static int load_policy(struct policy_data *policy)
> +{
> + return 0;
> +}
> +
> +static struct policy_data *create_policy(const char *ident)
> +{
> + struct policy_data *policy;
> + char *key;
> +
> + DBG("ident %s", ident);
> +
> + if (g_hash_table_lookup(ident_hash, ident) != NULL)
> + return NULL;
> +
> + policy = g_try_new0(struct policy_data, 1);
> + if (policy == NULL)
> + return NULL;
> +
> + key = g_strdup(ident);
> + policy->ident = g_strdup(ident);
> + policy->config = new_config();
> +
> + if (key == NULL || policy->ident == NULL || policy->config == NULL) {
> + g_free(key);
> + cleanup_policy(policy);
> + return NULL;
> + }
> +
> + g_hash_table_replace(ident_hash, key, policy);
> +
> + connman_info("Adding configuration %s", ident);
> +
> + return policy;
> +}
> +
> +static connman_bool_t validate_ident(const char *ident)
> +{
> + unsigned int i;
> +
> + if (ident == NULL)
> + return FALSE;
> +
> + for (i = 0; i < strlen(ident); i++)
> + if (g_ascii_isprint(ident[i]) == FALSE)
> + return FALSE;
> +
> + return TRUE;
> +}
> +
> +static int read_policies(void)
> +{
> + GDir *dir;
> +
> + DBG("");
> +
> + dir = g_dir_open(STORAGEDIR, 0, NULL);
Use another directory for this, at least to avoid cluttering up the
filesystem. Maybe STORAGEDIR/session_policy_ivi/ or something, as alrady
the .policy suffix doesn't seem to be the explicit property of only
session policy ivi plugin.
Please also give a thought on who needs to be able to
write/modify/delete the policy files! Is it some other process with
other permissions than what ConnMan has. If so, it that process might
not even have enough permissions to read the contents of STORAGEDIR in
the first place.
> + if (dir != NULL) {
> + const gchar *file;
> +
> + while ((file = g_dir_read_name(dir)) != NULL) {
> + GString *str;
> + gchar *ident;
> +
> + if (g_str_has_suffix(file, ".policy") == FALSE)
> + continue;
> +
> + ident = g_strrstr(file, ".policy");
> + if (ident == NULL)
> + continue;
> +
> + str = g_string_new_len(file, ident - file);
> + if (str == NULL)
> + continue;
> +
> + ident = g_string_free(str, FALSE);
> +
> + if (validate_ident(ident) == TRUE) {
> + struct policy_data *policy;
> +
> + policy = create_policy(ident);
> + if (policy != NULL)
> + load_policy(policy);
> + } else {
> + connman_error("Invalid config ident %s", ident);
> + }
> + g_free(ident);
> + }
> +
> + g_dir_close(dir);
> + }
> +
> + return 0;
> +}
> +
> static int session_policy_ivi_init(void)
> {
> int err;
> @@ -194,9 +293,25 @@ static int session_policy_ivi_init(void)
> goto err;
> }
>
> + ident_hash = g_hash_table_new_full(g_str_hash, g_str_equal,
> + g_free, NULL);
> + if (ident_hash == NULL) {
> + err = -ENOMEM;
> +
Re: [PATCH v1 19/24] session_policy_ivi: Factor out SELinux context parser
Hi,
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> From: Daniel Wagner
>
> ---
> plugins/session_policy_ivi.c | 36
> 1 file changed, 28 insertions(+), 8 deletions(-)
>
> diff --git a/plugins/session_policy_ivi.c b/plugins/session_policy_ivi.c
> index dd3bda0..70889ec 100644
> --- a/plugins/session_policy_ivi.c
> +++ b/plugins/session_policy_ivi.c
> @@ -90,20 +90,40 @@ static struct connman_session_config *new_config(void)
> return config;
> }
>
> +static char *parse_ident(unsigned char *context, unsigned int size)
> +{
> + char *str, *ident, **tokens;
> +
> + str = g_strndup((const gchar*)context, (gsize) size);
> + if (str == NULL)
> + return NULL;
> +
> + DBG("SELinux context %s", str);
> +
> + tokens = g_strsplit(str, ":", 0);
> + if (tokens == NULL) {
> + g_free(str);
> + return NULL;
> + }
> +
> + ident = g_strdup(tokens[2]);
> + g_strfreev(tokens);
> +
> + DBG("ident: %s", ident);
> +
> + return ident;
> +}
This part should have been added earlier, no need to have a separate
patch to factor it out.
The function above gets interesting. The commit message really should
explain how the ident is created via the SELinux security context.
> +
> static void selinux_context_reply(unsigned char *context, unsigned int size,
> void *user_data)
> {
> struct create_data *data = user_data;
> struct policy_data *policy = NULL;
> - char *str;
> + char *ident;
>
> DBG("session %p", data->session);
>
> - str = g_strndup((const gchar*)context, (gsize) size);
> - if (str == NULL)
> - goto err;
> -
> - DBG("SELinux context %s", str);
> + ident = parse_ident(context, size);
>
> policy = g_try_new0(struct policy_data, 1);
> if (policy == NULL)
> @@ -118,7 +138,7 @@ static void selinux_context_reply(unsigned char *context,
> unsigned int size,
> (*data->callback)(data->session, policy->config, data->user_data);
>
> g_free(data);
> - g_free(str);
> + g_free(ident);
>
> return;
> err:
> @@ -127,7 +147,7 @@ err:
> cleanup_policy(policy);
>
> g_free(data);
> - g_free(str);
> + g_free(ident);
> }
>
> static int policy_ivi_create(struct connman_session *session,
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Re: [PATCH v1 21/24] session: Export session parsing functions
On ons, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> From: Daniel Wagner
>
> ---
> include/session.h | 4 +++
> src/session.c | 76
> +--
> 2 files changed, 56 insertions(+), 24 deletions(-)
>
> diff --git a/include/session.h b/include/session.h
> index afad9fb..9b5a1f0 100644
> --- a/include/session.h
> +++ b/include/session.h
> @@ -87,6 +87,10 @@ GSList *connman_session_allowed_bearers_any(void);
> void connman_session_free_bearers(GSList *bearers);
> struct connman_session_config *connman_session_create_default_config(void);
>
> +enum connman_session_roaming_policy
> connman_session_parse_roaming_policy(const char *policy);
> +enum connman_session_type connman_session_parse_connection_type(const char
> *type);
> +int connman_session_parse_allowed_bearers(const char *token, GSList **list);
Functionality for connman_session_parse_allowed_bearers already exists
in src/main.c as the function parse_service_types(), but that one does
not interpret the '*' wildcard.
> +
> const char *connman_session_get_owner(struct connman_session *session);
>
> #ifdef __cplusplus
> diff --git a/src/session.c b/src/session.c
> index b39dd0a..d13c26b 100644
> --- a/src/session.c
> +++ b/src/session.c
> @@ -160,7 +160,23 @@ static const char *type2string(enum connman_session_type
> type)
> return NULL;
> }
>
> -static enum connman_session_type string2type(const char *type)
> +enum connman_session_roaming_policy
> connman_session_parse_roaming_policy(const char *policy)
> +{
> + if (g_strcmp0(policy, "default") == 0)
> + return CONNMAN_SESSION_ROAMING_POLICY_DEFAULT;
> + else if (g_strcmp0(policy, "always") == 0)
> + return CONNMAN_SESSION_ROAMING_POLICY_ALWAYS;
> + else if (g_strcmp0(policy, "forbidden") == 0)
> + return CONNMAN_SESSION_ROAMING_POLICY_FORBIDDEN;
> + else if (g_strcmp0(policy, "national") == 0)
> + return CONNMAN_SESSION_ROAMING_POLICY_NATIONAL;
> + else if (g_strcmp0(policy, "international") == 0)
> + return CONNMAN_SESSION_ROAMING_POLICY_INTERNATIONAL;
> + else
> + return CONNMAN_SESSION_ROAMING_POLICY_UNKNOWN;
> +}
> +
> +enum connman_session_type connman_session_parse_connection_type(const char
> *type)
> {
> if (g_strcmp0(type, "any") == 0)
> return CONNMAN_SESSION_TYPE_ANY;
Hmm, a service is 'ready' or 'online' while a session is 'local' or
'internet'. Maybe that should be fixed to reduce confusion... Especially
when it involves humans an text files.
> @@ -388,38 +404,49 @@ void connman_session_free_bearers(GSList *bearers)
> g_slist_free_full(bearers, cleanup_bearer);
> }
>
> +int connman_session_parse_allowed_bearers(const char *token, GSList **list)
> +{
> + struct connman_session_bearer *info;
> +
> + info = g_try_new0(struct connman_session_bearer, 1);
> + if (info == NULL) {
> + connman_session_free_bearers(*list);
> + *list = NULL;
> + return -ENOMEM;
> + }
> +
> + info->name = g_strdup(token);
> + info->service_type = bearer2service(info->name);
> +
> + if (info->service_type == CONNMAN_SERVICE_TYPE_UNKNOWN &&
> + g_strcmp0(info->name, "*") == 0) {
> + info->match_all = TRUE;
> + } else {
> + info->match_all = FALSE;
> + }
> +
> + *list = g_slist_append(*list, info);
> +
> + return 0;
> +}
> +
> static int session_parse_allowed_bearers(DBusMessageIter *iter, GSList
> **list)
> {
> - struct connman_session_bearer *bearer;
> DBusMessageIter array;
> + int err;
>
> dbus_message_iter_recurse(iter, &array);
>
> *list = NULL;
>
> while (dbus_message_iter_get_arg_type(&array) == DBUS_TYPE_STRING) {
> - char *bearer_name = NULL;
> -
> - dbus_message_iter_get_basic(&array, &bearer_name);
> -
> - bearer = g_try_new0(struct connman_session_bearer, 1);
> - if (bearer == NULL) {
> - connman_session_free_bearers(*list);
> - *list = NULL;
> - return -ENOMEM;
> - }
> + char *bearer = NULL;
>
> - bearer->name = g_strdup(bearer_name);
> - bearer->service_type = bearer2service(bearer->name);
> + dbus_message_iter_get_basic(&array, &bearer);
>
> - if (bearer->service_type == CONNMAN_SERVICE_TYPE_UNKNOWN &&
> - g_strcmp0(bearer->name, "*") == 0) {
> - bearer->match_all = TRUE;
> - } else {
> - bearer->match_all = FALSE;
> - }
> -
> - *list = g_slist_append(*list, bearer);
> + err = connman_session_parse_allowed_bearers(bearer, list);
> + if (err < 0)
> + return err;
>
> dbus_message_iter_next(&array);
>
Re: [PATCH v1 22/24] session_policy_ivi: Implement policy_load()
On Wed, 2012-10-17 at 14:42 +0200, Daniel Wagner wrote:
> From: Daniel Wagner
>
> ---
> plugins/session_policy_ivi.c | 94
> +++-
> 1 file changed, 92 insertions(+), 2 deletions(-)
>
> diff --git a/plugins/session_policy_ivi.c b/plugins/session_policy_ivi.c
> index c4e0e42..d52b345 100644
> --- a/plugins/session_policy_ivi.c
> +++ b/plugins/session_policy_ivi.c
> @@ -58,6 +58,8 @@ struct policy_data {
>
> static void cleanup_policy(struct policy_data *policy)
> {
> + DBG("policy %p", policy);
> +
> if (policy == NULL)
> return;
>
> @@ -212,9 +214,96 @@ static struct connman_session_policy session_policy_ivi
> = {
> .destroy = policy_ivi_destroy,
> };
>
> +static GKeyFile *load_keyfile(const char *pathname)
> +{
> + GKeyFile *keyfile = NULL;
> + GError *error = NULL;
> +
> + DBG("Loading %s", pathname);
> +
> + keyfile = g_key_file_new();
> +
> + if (!g_key_file_load_from_file(keyfile, pathname, 0, &error)) {
> + DBG("Unable to load %s: %s", pathname, error->message);
> + g_clear_error(&error);
> +
> + g_key_file_free(keyfile);
> + keyfile = NULL;
> + }
> +
> + return keyfile;
> +}
> +
> static int load_policy(struct policy_data *policy)
> {
> - return 0;
> + struct connman_session_config *config = policy->config;
> + GKeyFile *keyfile;
> + char *pathname;
> + char *str, **tokens;
> + int i, err = 0;
> +
> + pathname = g_strdup_printf("%s/%s.policy", STORAGEDIR, policy->ident);
> + if(pathname == NULL)
> + return -ENOMEM;
> +
> + keyfile = load_keyfile(pathname);
> + if (keyfile == NULL) {
> + g_free(pathname);
> + return -ENOMEM;
> + }
> +
> + config->priority = g_key_file_get_boolean(keyfile, "Default",
> + "Priority", NULL);
> +
> + str = g_key_file_get_string(keyfile, "Default", "RoamingPolicy",
> + NULL);
> + if (str != NULL) {
> + config->roaming_policy =
> connman_session_parse_roaming_policy(str);
> + g_free(str);
> + } else {
> + config->roaming_policy = CONNMAN_SESSION_ROAMING_POLICY_DEFAULT;
> + }
> +
> + str = g_key_file_get_string(keyfile, "Default", "ConnectionType",
> + NULL);
> + if (str != NULL) {
> + config->type = connman_session_parse_connection_type(str);
> + g_free(str);
> + } else {
> + config->type = CONNMAN_SESSION_TYPE_ANY;
> + }
> +
> + config->ecall = g_key_file_get_boolean(keyfile, "Default",
> + "EmergencyCall", NULL);
> +
When we read the ecall variable from a file or later via some other
policy entity over D-Bus, maybe the common code in session.c would
immediately disconnect all other sessions and establish only this
particular one? Or even better, the ecall variable need not be handled
in session.c, we have a hash of existing sessions so let's just close
all of them and (implement and) call set_emergency_mode(TRUE). If the
emergency session ever goes away, set_emergency_mode(FALSE) and normal
operation can continue. If we have several policies, is one ecall
notification enough...?
Just a thought to be considered later.
> + connman_session_free_bearers(config->allowed_bearers);
> + config->allowed_bearers = NULL;
> +
> + str = g_key_file_get_string(keyfile, "Default", "AllowedBearers",
> + NULL);
> +
> + if (str != NULL) {
> + tokens = g_strsplit(str, " ", 0);
> +
> + for (i = 0; tokens[i] != NULL; i++) {
> + err = connman_session_parse_allowed_bearers(tokens[i],
> + &config->allowed_bearers);
> + if (err < 0)
> + break;
> + }
> +
> + g_free(str);
> + g_strfreev(tokens);
> + } else {
> + config->allowed_bearers = connman_session_allowed_bearers_any();
> + if (config->allowed_bearers == NULL)
> + err = -ENOMEM;
> + }
> +
> + g_key_file_free(keyfile);
> + g_free(pathname);
> +
> + return err;
> }
>
> static struct policy_data *create_policy(const char *ident)
> @@ -293,7 +382,8 @@ static void notify_handler(struct inotify_event *event,
> if (policy != NULL) {
> load_policy(policy);
>
> - connman_session_config_update(policy->session);
> + if (policy->session != NULL)
> + connman_session_config_update(policy->session);
> }
> }
>
___
connman mailing list
connman@connman.net
http://lists.connman.net/listinfo/connman
Please give the latest ConnMan a spin
Hi there, There are quite a few fixes in git added after the latest ConnMan release. It would be nice if people could try out the latest and greatest and report back if any issues are found. Cheers, Patrik ___ connman mailing list connman@connman.net http://lists.connman.net/listinfo/connman
