[RFC v0] session: Use bits for allowed_bearers instead a list

Daniel Wagner wagi at monom.org
Wed Nov 14 04:49:28 PST 2012


From: Daniel Wagner <daniel.wagner at bmw-carit.de>

At this point we support at max 10 different technologies. So there
is no real need to have a list for allowed_bearers.

This simplifies apply_policy_on_bearers dramtically. We only need
a simple bitwise AND operation to figure out what is allowed
and what not.
---

Hi, 

I think this something we should do instead of the list. Obviously,
this should also be applied to struct connman_setttings. No need
to have an uint8 array for this.

What do you think?

cheers,
daniel


 include/session.h |   2 +-
 src/session.c     | 153 ++++++++++++++++--------------------------------------
 2 files changed, 46 insertions(+), 109 deletions(-)

diff --git a/include/session.h b/include/session.h
index b93fbc7..21d94a8 100644
--- a/include/session.h
+++ b/include/session.h
@@ -56,7 +56,7 @@ struct connman_session_config {
 	enum connman_session_roaming_policy roaming_policy;
 	enum connman_session_type type;
 	connman_bool_t ecall;
-	GSList *allowed_bearers;
+	unsigned int allowed_bearers;
 };
 
 typedef void (* connman_session_config_cb) (struct connman_session *session,
diff --git a/src/session.c b/src/session.c
index 77a6c85..bc7841a 100644
--- a/src/session.c
+++ b/src/session.c
@@ -96,6 +96,19 @@ struct connman_session {
 	GHashTable *service_hash;
 };
 
+#define BIT(x) (1 << (x))
+
+static unsigned int find_next_bit(unsigned int p, unsigned int *x)
+{
+	*x &= ~BIT(p - 1);
+	return __builtin_ffs(*x);
+}
+
+#define for_each_set_bit(p, x)		\
+	for (p = __builtin_ffs(x);	\
+	     p > 0;			\
+	     p = find_next_bit(p, &x))
+
 static const char *trigger2string(enum connman_session_trigger trigger)
 {
 	switch (trigger) {
@@ -348,12 +361,7 @@ struct connman_session_config *connman_session_create_default_config(void)
 	config->roaming_policy = CONNMAN_SESSION_ROAMING_POLICY_DEFAULT;
 	config->type = CONNMAN_SESSION_TYPE_ANY;
 	config->ecall = FALSE;
-	config->allowed_bearers = g_slist_prepend(NULL,
-				GINT_TO_POINTER(CONNMAN_SERVICE_TYPE_UNKNOWN));
-	if (config->allowed_bearers == NULL) {
-		g_free(config);
-		return NULL;
-	}
+	config->allowed_bearers = BIT(CONNMAN_SERVICE_TYPE_UNKNOWN);
 
 	return config;
 }
@@ -374,7 +382,7 @@ static enum connman_session_type apply_policy_on_type(
 	return CONNMAN_SESSION_TYPE_INTERNET;
 }
 
-static int parse_bearers(DBusMessageIter *iter, GSList **list)
+static int parse_bearers(DBusMessageIter *iter, unsigned int *bearers)
 {
 	enum connman_service_type bearer;
 	DBusMessageIter array;
@@ -382,15 +390,14 @@ static int parse_bearers(DBusMessageIter *iter, GSList **list)
 
 	dbus_message_iter_recurse(iter, &array);
 
-	*list = NULL;
+	*bearers = 0;
 
 	while ((type = dbus_message_iter_get_arg_type(&array)) !=
 			DBUS_TYPE_INVALID) {
 		char *bearer_name = NULL;
 
 		if (type != DBUS_TYPE_STRING) {
-			g_slist_free(*list);
-			*list = NULL;
+			*bearers = 0;
 			return -EINVAL;
 		}
 
@@ -406,12 +413,11 @@ static int parse_bearers(DBusMessageIter *iter, GSList **list)
 
 		err = bearer2service(bearer_name, &bearer);
 		if (err < 0) {
-			g_slist_free(*list);
-			*list = NULL;
+			*bearers = 0;
 			return err;
 		}
 
-		*list = g_slist_append(*list, GINT_TO_POINTER(bearer));
+		*bearers |= BIT(bearer);
 
 	next:
 		dbus_message_iter_next(&array);
@@ -420,63 +426,23 @@ static int parse_bearers(DBusMessageIter *iter, GSList **list)
 	return 0;
 }
 
-static int filter_bearer(GSList *policy_bearers,
-				enum connman_service_type bearer,
-				GSList **list)
-{
-	enum connman_service_type policy;
-	GSList *it;
-
-	if (policy_bearers == NULL)
-		goto clone;
-
-	for (it = policy_bearers; it != NULL; it = it->next) {
-		policy = GPOINTER_TO_INT(it->data);
-
-		if (policy != CONNMAN_SERVICE_TYPE_UNKNOWN && policy != bearer)
-			continue;
-
-		goto clone;
-	}
-
-	*list = NULL;
-
-	return 0;
-
-clone:
-	*list = g_slist_append(*list, GINT_TO_POINTER(bearer));
-
-	return 0;
-}
-
-static int apply_policy_on_bearers(GSList *policy_bearers, GSList *bearers,
-				GSList **list)
+static unsigned int apply_policy_on_bearers(unsigned int policy_bearers,
+						unsigned int bearers)
 {
-	enum connman_service_type bearer;
-	GSList *it;
-	int err;
+	if (policy_bearers & BIT(CONNMAN_SERVICE_TYPE_UNKNOWN))
+		return bearers;
 
-	*list = NULL;
-
-	for (it = bearers; it != NULL; it = it->next) {
-		bearer = GPOINTER_TO_INT(it->data);
-
-		err = filter_bearer(policy_bearers, bearer, list);
-		if (err < 0)
-			return err;
-	}
-
-	return 0;
+	return policy_bearers & bearers;
 }
 
 static void append_allowed_bearers(DBusMessageIter *iter, void *user_data)
 {
 	struct session_info *info = user_data;
-	GSList *list;
+	unsigned int p, bearers;
 
-	for (list = info->config.allowed_bearers;
-			list != NULL; list = list->next) {
-		enum connman_service_type bearer = GPOINTER_TO_INT(list->data);
+	bearers = info->config.allowed_bearers;
+	for_each_set_bit(p, bearers) {
+		enum connman_service_type bearer = p - 1;
 		const char *name = __connman_service_type2string(bearer);
 
 		if (name == NULL)
@@ -699,15 +665,14 @@ static void ipconfig_ipv6_changed(struct connman_session *session)
 						info->entry->service);
 }
 
-static connman_bool_t service_type_match(struct connman_session *session,
+static connman_bool_t service_match(struct connman_session *session,
 					struct connman_service *service)
 {
-	struct session_info *info = session->info;
-	GSList *list;
+	unsigned int p, bearers;
 
-	for (list = info->config.allowed_bearers;
-			list != NULL; list = list->next) {
-		enum connman_service_type bearer = GPOINTER_TO_INT(list->data);
+	bearers = session->info->config.allowed_bearers;
+	for_each_set_bit(p, bearers) {
+		enum connman_service_type bearer = p - 1;
 		enum connman_service_type service_type;
 
 		if (bearer == CONNMAN_SERVICE_TYPE_UNKNOWN)
@@ -721,15 +686,6 @@ static connman_bool_t service_type_match(struct connman_session *session,
 	return FALSE;
 }
 
-static connman_bool_t service_match(struct connman_session *session,
-					struct connman_service *service)
-{
-	if (service_type_match(session, service) == FALSE)
-		return FALSE;
-
-	return TRUE;
-}
-
 static int service_type_weight(enum connman_service_type type)
 {
 	/*
@@ -769,16 +725,16 @@ static gint sort_allowed_bearers(struct connman_service *service_a,
 					struct connman_session *session)
 {
 	struct session_info *info = session->info;
-	GSList *list;
 	enum connman_service_type type_a, type_b;
 	int weight_a, weight_b;
+	unsigned int p, bearers;
 
 	type_a = connman_service_get_type(service_a);
 	type_b = connman_service_get_type(service_b);
 
-	for (list = info->config.allowed_bearers;
-			list != NULL; list = list->next) {
-		enum connman_service_type bearer = GPOINTER_TO_INT(list->data);
+	bearers = info->config.allowed_bearers;
+	for_each_set_bit(p, bearers) {
+		enum connman_service_type bearer = p - 1;
 
 		if (bearer == CONNMAN_SERVICE_TYPE_UNKNOWN) {
 			if (type_a != type_b) {
@@ -824,7 +780,6 @@ static void free_session(struct connman_session *session)
 		return;
 
 	destroy_policy_config(session);
-	g_slist_free(session->info->config.allowed_bearers);
 	g_free(session->owner);
 	g_free(session->session_path);
 	g_free(session->notify_path);
@@ -1385,9 +1340,9 @@ static DBusMessage *change_session(DBusConnection *conn,
 	struct connman_session *session = user_data;
 	struct session_info *info = session->info;
 	DBusMessageIter iter, value;
+	unsigned int allowed_bearers;
 	const char *name;
 	const char *val;
-	GSList *allowed_bearers;
 	int err;
 
 	DBG("session %p", session);
@@ -1412,15 +1367,9 @@ static DBusMessage *change_session(DBusConnection *conn,
 			if (err < 0)
 				return __connman_error_failed(msg, err);
 
-			g_slist_free(info->config.allowed_bearers);
-			err = apply_policy_on_bearers(
+			info->config.allowed_bearers = apply_policy_on_bearers(
 					session->policy_config->allowed_bearers,
-					allowed_bearers,
-					&info->config.allowed_bearers);
-
-			g_slist_free(allowed_bearers);
-			if (err < 0)
-				return __connman_error_failed(msg, err);
+					allowed_bearers);
 		} else {
 			goto err;
 		}
@@ -1528,7 +1477,7 @@ struct user_config {
 	DBusMessage *pending;
 
 	enum connman_session_type type;
-	GSList *allowed_bearers;
+	unsigned int allowed_bearers;
 };
 
 static void session_create_cb(struct connman_session *session,
@@ -1560,12 +1509,9 @@ static void session_create_cb(struct connman_session *session,
 	info->config.roaming_policy = session->policy_config->roaming_policy;
 	info->entry = NULL;
 
-	err = apply_policy_on_bearers(
+	info->config.allowed_bearers = apply_policy_on_bearers(
 			session->policy_config->allowed_bearers,
-			user_config->allowed_bearers,
-			&info->config.allowed_bearers);
-	if (err < 0)
-		goto out;
+			user_config->allowed_bearers);
 
 	g_hash_table_replace(session_hash, session->session_path, session);
 
@@ -1605,7 +1551,6 @@ out:
 	}
 
 	dbus_message_unref(user_config->pending);
-	g_slist_free(user_config->allowed_bearers);
 	g_free(user_config);
 }
 
@@ -1688,15 +1633,8 @@ int __connman_session_create(DBusMessage *msg)
 	 *
 	 * For AllowedBearers this is '*', ...
 	 */
-	if (user_allowed_bearers == FALSE) {
-		user_config->allowed_bearers =
-			g_slist_append(NULL,
-				GINT_TO_POINTER(CONNMAN_SERVICE_TYPE_UNKNOWN));
-		if (user_config->allowed_bearers == NULL) {
-			err = -ENOMEM;
-			goto err;
-		}
-	}
+	if (user_allowed_bearers == FALSE)
+		user_config->allowed_bearers = BIT(CONNMAN_SERVICE_TYPE_UNKNOWN);
 
 	/* ... and for ConnectionType it is 'any'. */
 	if (user_connection_type == FALSE)
@@ -1768,7 +1706,6 @@ err:
 
 	if (user_config != NULL) {
 		dbus_message_unref(user_config->pending);
-		g_slist_free(user_config->allowed_bearers);
 		g_free(user_config);
 	}
 	return err;
-- 
1.8.0.rc0




More information about the connman mailing list