Send connman mailing list submissions to
[email protected]
To subscribe or unsubscribe via email, send a message with subject or
body 'help' to
[email protected]
You can reach the person managing the list at
[email protected]
When replying, please edit your Subject line so it is more specific
than "Re: Contents of connman digest..."
Today's Topics:
1. Re: [PATCH 1/2] ofono: set network name from context name
(Daniel Wagner)
----------------------------------------------------------------------
Date: Wed, 4 Mar 2020 08:51:50 +0100
From: Daniel Wagner <[email protected]>
Subject: Re: [PATCH 1/2] ofono: set network name from context name
To: Nicola Lunghi <[email protected]>
Cc: connman <[email protected]>
Message-ID: <[email protected]>
Content-Type: text/plain; charset=us-ascii
Hi Nicola,
On Tue, Mar 03, 2020 at 06:27:35PM +0000, Nicola Lunghi wrote:
> previously connman was setting the network name using the netreg Name property
Previously ConnMan ...
> that value doesn't permit to know what context we are connected to.
>
> Set the network name from the context name instead
Could you please explain in the commit message why we don't need to
update the name netreg_properties_reply() callback? I'd like to
understand why it's safe to it later. I suppose the network hasn't
been registered to the core at this point? Though the updates from
oFono are not strictly ordered. So I am bit concerned that it's not
always working.
> Signed-off-by: Nicola Lunghi <[email protected]>
We don't do the SoB.
> ---
> plugins/ofono.c | 79 ++++++++++++++++++++++++-------------------------
> 1 file changed, 38 insertions(+), 41 deletions(-)
>
> diff --git a/plugins/ofono.c b/plugins/ofono.c
> index 82413b6..bb08cfd 100644
> --- a/plugins/ofono.c
> +++ b/plugins/ofono.c
> @@ -132,6 +132,7 @@ static GHashTable *context_hash;
>
> struct network_context {
> char *path;
> + char *name;
> int index;
> struct connman_network *network;
>
> @@ -293,6 +294,7 @@ static void network_context_unref(struct network_context
> *context)
> return;
>
> g_free(context->path);
> + g_free(context->name);
>
> connman_ipaddress_free(context->ipv4_address);
> g_free(context->ipv4_nameservers);
> @@ -1116,7 +1118,10 @@ static void add_network(struct modem_data *modem,
> connman_network_set_string(context->network, "Path",
> context->path);
>
> - if (modem->name)
> + /* we try first to use the context name as name, then the modem name */
No need for such a comment. It's very clear from the code what's going on.
> + if (context->name)
> + connman_network_set_name(context->network, context->name);
> + else if (modem->name)
> connman_network_set_name(context->network, modem->name);
> else
> connman_network_set_name(context->network, "");
> @@ -1152,6 +1157,23 @@ static void remove_network(struct modem_data *modem,
> context->network = NULL;
> }
>
> +static int set_context_name(struct network_context *context,
> + const char *name)
> +{
> + if (!context || !name)
> + return -EINVAL;
> +
> + g_free(context->name);
> + context->name = g_strdup(name);
> +
> + if(!context->name)
> + return -ENOMEM;
> +
> + DBG("%s Set context name to %s", context->path, context->name);
> +
> + return 0;
> +}
> +
> static int set_context_ipconfig(struct network_context *context,
> const char *protocol)
> {
> @@ -1199,6 +1221,7 @@ static int add_cm_context(struct modem_data *modem,
> const char *context_path,
> struct network_context *context = NULL;
> dbus_bool_t active = FALSE;
> const char *ip_protocol = NULL;
> + const char *ctx_name = NULL;
>
> DBG("%s context path %s", modem->path, context_path);
>
> @@ -1216,7 +1239,13 @@ static int add_cm_context(struct modem_data *modem,
> const char *context_path,
> dbus_message_iter_next(&entry);
> dbus_message_iter_recurse(&entry, &value);
>
> - if (g_str_equal(key, "Type")) {
> + if (g_str_equal(key, "Name") &&
> + dbus_message_iter_get_arg_type(&value) ==
> DBUS_TYPE_STRING ) {
> + dbus_message_iter_get_basic(&value, &ctx_name);
> +
> + DBG("%s Got Context Name %s", modem->path, ctx_name);
> +
> + } else if (g_str_equal(key, "Type")) {
> dbus_message_iter_get_basic(&value, &context_type);
>
> DBG("%s context %s type %s", modem->path,
> @@ -1259,6 +1288,9 @@ static int add_cm_context(struct modem_data *modem,
> const char *context_path,
> return -EINVAL;
> }
>
> + if (ctx_name)
> + set_context_name(context, ctx_name);
> +
> if (ip_protocol)
> set_context_ipconfig(context, ip_protocol);
>
> @@ -1571,33 +1603,6 @@ static gboolean cm_context_removed(DBusConnection
> *conn,
> return TRUE;
> }
>
> -static void netreg_update_name(struct modem_data *modem,
> - DBusMessageIter* value)
> -{
> - char *name;
> - GSList *list;
> -
> - dbus_message_iter_get_basic(value, &name);
> -
> - DBG("%s Name %s", modem->path, name);
> -
> - g_free(modem->name);
> - modem->name = g_strdup(name);
> -
> - if (!modem->context_list)
> - return;
> -
> - /* For all the context */
> - for (list = modem->context_list; list; list = list->next) {
> - struct network_context *context = list->data;
> -
> - if (context->network) {
> - connman_network_set_name(context->network, modem->name);
> - connman_network_update(context->network);
> - }
> - }
> -}
I am worried about this part here. IIRC the netreg update can happen
at any time. That means the function might not be called always in the
same order. And I think this is the reason we have this loop is here.
Thanks,
Daniel
------------------------------
Subject: Digest Footer
_______________________________________________
connman mailing list -- [email protected]
To unsubscribe send an email to [email protected]
------------------------------
End of connman Digest, Vol 53, Issue 4
**************************************
