Hi,
After the colon we start with capital letter. The D-Bus function name is
called CreateSession and finally the Session API is more or less
consistently written with capital letter. Yes, this doesn't sound like a
big deal and I usually would fix it up myself. But there are more things
no okay in this patch.
On 05/22/2017 07:12 AM, sutar.mounesh(a)gmail.com wrote:
From: Bjoern Thorwirth
<external.bjoern.thorwirth(a)de.bosch.com>
Extend the session API interface. enable a service differentiation
for processes run by the same user. allow ConnMan to differentiate between bearer
usage permissions and the respective priorities based on the requested service type.
Usually calling process that implements the session API is identified by the user ID
as it is runs. All processes of the same user share the same list of allowed bearers,
and the same priority for choosing between available bearers is applied.
This extension allows processes to select a service context
for which the routing decision is made.
This commit message should explain why this change is necessary at the
first thing. I would make the second paragraph as the first one.
Since this is an official D-Bus API I want to have proper written commit
message for this. Please take the time to write one.
---
doc/session-api.txt | 11 +++++++++++
include/session.h | 1 +
src/session.c | 17 +++++++++++++++++
3 files changed, 29 insertions(+)
diff --git a/doc/session-api.txt b/doc/session-api.txt
index e8da522..a449af2 100644
--- a/doc/session-api.txt
+++ b/doc/session-api.txt
@@ -205,3 +205,14 @@ Settings string State [readonly]
a default route. When the source IP rule is enabled,
an application can select which session/interface to
send traffic on, using bind-before-connect mechanism.
+
+ string Service [readonly]
readonly? According the code you can write it too.
+
+ The service context of the Session.
What does that mean?
+ Usually calling process that implements the session
API is identified
+ by the user ID as it is runs. All processes of the same user share the
same list of
+ allowed bearers, and the same priority for choosing between available
+ bearers is applied.
+ This parameter can be used to implement service context dependent
behavior
+ In session_policy_local.c. It should be supplied with the “dict
Settings” parameter
+ supplied with CreateSession() call from within the manager-api.
The line are too long. Check the formatting of the whole document.
Also take a closer look how the documentation is written on the rest.
There should also no reference to session_policy.c. You don't have to
explain in detail what is for, e.g listing a example. You should more
details in the session-overview.txt document.
diff --git a/include/session.h b/include/session.h
index 5106e88..3b3c53f 100644
--- a/include/session.h
+++ b/include/session.h
@@ -74,6 +74,7 @@ struct connman_session_config {
GSList *allowed_bearers;
char *allowed_interface;
bool source_ip_rule;
+ char *service;
};
typedef int (* connman_session_config_func_t) (struct connman_session *session,
diff --git a/src/session.c b/src/session.c
index 9dd183f..2437538 100644
--- a/src/session.c
+++ b/src/session.c
@@ -548,6 +548,7 @@ struct creation_data {
GSList *allowed_bearers;
char *allowed_interface;
bool source_ip_rule;
+ char *service;
};
static void cleanup_creation_data(struct creation_data *creation_data)
@@ -557,6 +558,8 @@ static void cleanup_creation_data(struct creation_data
*creation_data)
if (creation_data->pending)
dbus_message_unref(creation_data->pending);
+ if (creation_data->service)
+ g_free(creation_data->service);
g_slist_free(creation_data->allowed_bearers);
g_free(creation_data->allowed_interface);
@@ -927,6 +930,17 @@ static void append_notify(DBusMessageIter *dict,
}
if (session->append_all ||
+ info->config.service != info_last->config.service) {
+ char *ifname = info->config.service;
+ if (!ifname)
+ ifname = "";
+ connman_dbus_dict_append_basic(dict, "Service",
+ DBUS_TYPE_STRING,
+ &ifname);
+ info_last->config.service = info->config.service;
+ }
+
+ if (session->append_all ||
info->config.source_ip_rule != info_last->config.source_ip_rule) {
dbus_bool_t source_ip_rule = FALSE;
if (info->config.source_ip_rule)
@@ -1474,6 +1488,9 @@ int __connman_session_create(DBusMessage *msg)
connman_session_parse_connection_type(val);
user_connection_type = true;
+ } else if (g_str_equal(key, "Service")) {
+ dbus_message_iter_get_basic(&value, &val);
+ creation_data->service = g_strdup(val);
} else if (g_str_equal(key, "AllowedInterface")) {
dbus_message_iter_get_basic(&value, &val);
creation_data->allowed_interface = g_strdup(val);
There is also the connmanctl client, which needs attention.
And then there is also some sort of encoding problem, because git am
complains with:
error: cannot convert from Y to UTF-8
fatal: could not parse patch
A good trick is to send yourself the patch first and then try to apply
it with git am. If that doesn't work don't send it.
Thanks,
Daniel