mbox series

[v2,0/2] ofono: Correct conditional in 'cm_get_contexts_reply'.

Message ID cover.1739203086.git.gerickson@nuovations.com (mailing list archive)
Headers show
Series ofono: Correct conditional in 'cm_get_contexts_reply'. | expand

Message

Grant Erickson Feb. 10, 2025, 3:59 p.m. UTC
At the appropriate moment in the oFono plugin lifecycle, it gets and
evaluates contexts from a given Cellular modem. Depending on the
HNI/MCC+MNC and the network operator, there may be one or more such
contexts, each with a different type.

Within 'cm_get_contexts_reply', each received context is iterated on,
evaluating each for the type 'internet' by calling
'add_cm_context'. If the context is not of type 'internet',
'cm_get_contexts_reply' should continue to iterate until all contexts
are exhausted or a matching context is found.

The return semantics of 'add_cm_context' are '-ENOMEM' if memory could
not be allocated, '-EINVAL' if a context is *not* of the type
'internet'; otherwise zero ('0') if a context of type 'internet' was
found.

However, the conditional logic of 'cm_get_contexts_reply' prior to
this change was:

   while (dbus_message_iter_get_arg_type(&dict) == DBUS_TYPE_STRUCT) {
       const char *context_path;

       dbus_message_iter_recurse(&dict, &entry);
       dbus_message_iter_get_basic(&entry, &context_path);

       dbus_message_iter_next(&entry);
       dbus_message_iter_recurse(&entry, &value);

       if (add_cm_context(modem, context_path, &value))
           break;

       dbus_message_iter_next(&dict);
   }

So, assuming a set of contexts from, for example Verizon, such as '[ {
vzwims, ims }, { vzwinternet, internet }, { vzwapp, wap } ]', then the
above logic will encounter '{ vzwims, ims }' on the first iteration,
evaluate that it is not of type 'internet', return '-EINVAL' from
'add_cm_context', that will satisfy the conditional and trigger the
'break' from the 'while' loop and context iteration will
terminate. This then misses the second, desired '{ vzwinternet,
internet }' context and the Cellular interface will never come up.

By changing the conditional logic to match the return semantics of
'add_cm_context' the code now works correctly, regardless of the number
of contexts iterated on or their order.

Fixes: 4b238d8590942 ("ofono: Implementation of simultaneous APNS")

Grant Erickson (2):
  ofono: Correct conditional in 'cm_get_contexts_reply'.
  ofono: Document 'add_cm_context'.

 plugins/ofono.c | 35 ++++++++++++++++++++++++++++++++++-
 1 file changed, 34 insertions(+), 1 deletion(-)

Comments

patchwork-bot+connman@kernel.org Feb. 14, 2025, 4:10 p.m. UTC | #1
Hello:

This series was applied to connman.git (master)
by Denis Kenzior <denkenz@gmail.com>:

On Mon, 10 Feb 2025 07:59:47 -0800 you wrote:
> At the appropriate moment in the oFono plugin lifecycle, it gets and
> evaluates contexts from a given Cellular modem. Depending on the
> HNI/MCC+MNC and the network operator, there may be one or more such
> contexts, each with a different type.
> 
> Within 'cm_get_contexts_reply', each received context is iterated on,
> evaluating each for the type 'internet' by calling
> 'add_cm_context'. If the context is not of type 'internet',
> 'cm_get_contexts_reply' should continue to iterate until all contexts
> are exhausted or a matching context is found.
> 
> [...]

Here is the summary with links:
  - [v2,1/2] ofono: Correct conditional in 'cm_get_contexts_reply'.
    https://git.kernel.org/pub/scm/network/connman/connman.git/?id=28a7ce1595a1
  - [v2,2/2] ofono: Document 'add_cm_context'.
    https://git.kernel.org/pub/scm/network/connman/connman.git/?id=d543e126896f

You are awesome, thank you!