[PATCH 2/2] broadband-modem-mbim: fix preservation logic for PIN1 unlock retries

[Dan Williams]

On Mon, 2017-09-18 at 21:30 -0700, Aleksander Morgado wrote:
> On Mon, Sep 18, 2017 at 9:10 AM, Ben Chan <benchan at chromium.org>
> wrote:
> > This patches fixes commit 334273979 "broadband-modem-mbim: preserve
> > unlock retries for PIN1 when appropriate", which doesn't correctly
> > propagate the unlock retries information for PIN1 observed from
> > responses to MBIM_CID_PIN set operations (see commit eb9ec1b61
> > "sim-mbim: update unlock retries information after PIN
> > operations").
> > ---
> 
> LGTM
> 
> Dan, what do you think? Given that the PIN retries are updated in the
> iface from within different places (e.g. modem object and sim object)
> it probably isn't a good thing to cache the value anywhere else than
> in the iface itself. This is probably the simplest solution.

Yeah, you guys are right.  Both patches LGTM.

Dan

> >  src/mm-broadband-modem-mbim.c | 29 +++++++++++++----------------
> >  1 file changed, 13 insertions(+), 16 deletions(-)
> > 
> > diff --git a/src/mm-broadband-modem-mbim.c b/src/mm-broadband-
> > modem-mbim.c
> > index 93a61085..aaa3e1cc 100644
> > --- a/src/mm-broadband-modem-mbim.c
> > +++ b/src/mm-broadband-modem-mbim.c
> > @@ -93,9 +93,6 @@ struct _MMBroadbandModemMbimPrivate {
> > 
> >      /* For notifying when the mbim-proxy connection is dead */
> >      gulong mbim_device_removed_id;
> > -
> > -    /* Previously observed SIM-PIN remaining retries */
> > -    guint sim_pin_retries;
> >  };
> > 
> >  /*****************************************************************
> > ************/
> > @@ -713,7 +710,7 @@ pin_query_unlock_retries_ready (MbimDevice
> > *device,
> >              NULL,
> >              &remaining_attempts,
> >              &error)) {
> > -        MMBroadbandModemMbim *self;
> > +        MMIfaceModem *self;
> >          MMModemLock lock;
> >          MMUnlockRetries *retries;
> > 
> > @@ -737,22 +734,24 @@ pin_query_unlock_retries_ready (MbimDevice
> > *device,
> >           * PIN1. Here we thus carry over any existing information
> > on PIN1 from
> >           * MMIfaceModem's MMUnlockRetries if the MBIM_CID_PIN
> > query reports
> >           * something other than PIN1. */
> > -        if (lock != MM_MODEM_LOCK_SIM_PIN &&
> > -            self->priv->sim_pin_retries !=
> > MM_UNLOCK_RETRIES_UNKNOWN) {
> > -            mm_unlock_retries_set (retries,
> > -                                   MM_MODEM_LOCK_SIM_PIN,
> > -                                   self->priv->sim_pin_retries);
> > +        if (lock != MM_MODEM_LOCK_SIM_PIN) {
> > +            MMUnlockRetries *previous_retries;
> > +            guint previous_sim_pin_retries;
> > +
> > +            previous_retries = mm_iface_modem_get_unlock_retries
> > (self);
> > +            previous_sim_pin_retries = mm_unlock_retries_get
> > (previous_retries,
> > +                                                              MM_M
> > ODEM_LOCK_SIM_PIN);
> > +            if (previous_sim_pin_retries !=
> > MM_UNLOCK_RETRIES_UNKNOWN) {
> > +                mm_unlock_retries_set (retries,
> > +                                       MM_MODEM_LOCK_SIM_PIN,
> > +                                       previous_sim_pin_retries);
> > +            }
> >          }
> > 
> >          /* According to the MBIM specification, RemainingAttempts
> > is set to
> >           * 0xffffffff if the device does not support this
> > information. */
> >          if (remaining_attempts != G_MAXUINT32)
> >              mm_unlock_retries_set (retries, lock,
> > remaining_attempts);
> > -        else
> > -            remaining_attempts = MM_UNLOCK_RETRIES_UNKNOWN;
> > -
> > -        if (lock == MM_MODEM_LOCK_SIM_PIN)
> > -            self->priv->sim_pin_retries = remaining_attempts;
> > 
> >          g_task_return_pointer (task, retries, g_object_unref);
> >      } else
> > @@ -3274,8 +3273,6 @@ mm_broadband_modem_mbim_init
> > (MMBroadbandModemMbim *self)
> >      self->priv = G_TYPE_INSTANCE_GET_PRIVATE (self,
> >                                                MM_TYPE_BROADBAND_MO
> > DEM_MBIM,
> >                                                MMBroadbandModemMbim
> > Private);
> > -
> > -    self->priv->sim_pin_retries = MM_UNLOCK_RETRIES_UNKNOWN;
> >  }
> > 
> >  static void
> > --
> > 2.14.1.690.gbb1197296e-goog
> > 
> 
> 
>