Hi,

On Fri, 2015-10-09 at 08:37 +0200, Mylene JOSSERAND wrote:
> This is a first patch of a serie to implement a simultaneous APNs context 
> activation.
> 
> The current commit removes the previous implementation of one context per 
> modem and
> adds a list of contexts instead.

As this patch is really quite long, it looks it would be possible to add
GSList *context_list without removing struct network_context *context
immediately. So the code would function as before, but the contexts are
added/removed to/from the list. The next patch could then remove struct
network_context *context and change the functionality to always use the
list.

Instead of looking up the context with g_slist_nth_data and a
context_id, can't the code simply pass around a struct network_context *
all the time? For example in network_connect() the corresponding call
would then look like 'context_set_active(modem, context, TRUE)'. It's
much simpler that way and avoids iterating over the list twice. Right
now it is not immediately clear what a context_id with values 0 or -1
means.

Also, if you have a singly-linked list, please always prepend items as
the appending function has to always reach the end of the list before an
item is added. Since the order makes no difference here, prepeding is a
much more elegant (and faster) solution.

> Some modifications are necessary : some properties in the modem structure are 
> moved
> to the context structure (such as connman_network, valid_apn, etc) as they 
> are properties
> specifics to context.

Ok.

> Some functions are implemented to search for a specific context in the list 
> by his path
> or by his network. A function to remove all the context of one modem
> is also implemented.

Yes, those need to be implemented. The other two patches are really nice
and small.

Cheers,

        Patrik

_______________________________________________
connman mailing list
connman@connman.net
https://lists.connman.net/mailman/listinfo/connman

Reply via email to