[vlc-devel] [PATCH v3 01/11] keystore: refactor next order handling

[Thomas Guillem]

On Tue, Dec 22, 2020, at 09:07, Rémi Denis-Courmont wrote:
> Le maanantaina 21. joulukuuta 2020, 19.54.26 EET Thomas Guillem a écrit :
> > The order is now incremented before the processing.
> > 
> > No functional changes, will be used by the next commit.
> > ---
> >  include/vlc_keystore.h |  1 +
> >  src/misc/keystore.c    | 29 ++++++++++++++++++++---------
> >  2 files changed, 21 insertions(+), 9 deletions(-)
> > 
> > diff --git a/include/vlc_keystore.h b/include/vlc_keystore.h
> > index 80262987c28..a5c9e29faa2 100644
> > --- a/include/vlc_keystore.h
> > +++ b/include/vlc_keystore.h
> > @@ -177,6 +177,7 @@ struct vlc_credential
> > 
> >      /* internal */
> >      enum {
> > +        GET_FROM_INIT,
> >          GET_FROM_URL,
> >          GET_FROM_OPTION,
> >          GET_FROM_MEMORY_KEYSTORE,
> 
> This is strange. The caller ought to know if it needs any credentials, or if 
> it needs new credential because the previous ones fail(ed).
> 
> This looks like you are trying to second-guess the caller.

This patch doesn't change the current behavior. vlc_credential is a helper around the keystore. I noticed that all accesses used the same logic. First use the credential from the URL (but it's discouraged with a warning), then from a VLC option ("smb-user", "ftp-user"...), then from the keystore and finally from the dialog until credential is valid or the user cancel it.

> 
> > diff --git a/src/misc/keystore.c b/src/misc/keystore.c
> > index d58e8dea982..31aaf39e9ec 100644
> > --- a/src/misc/keystore.c
> > +++ b/src/misc/keystore.c
> > @@ -351,7 +351,7 @@ vlc_credential_init(vlc_credential *p_credential, const
> > vlc_url_t *p_url) assert(p_credential);
> > 
> >      memset(p_credential, 0, sizeof(*p_credential));
> > -    p_credential->i_get_order = GET_FROM_URL;
> > +    p_credential->i_get_order = GET_FROM_INIT;
> >      p_credential->p_url = p_url;
> >  }
> > 
> > @@ -371,6 +371,19 @@ vlc_credential_clean(vlc_credential *p_credential)
> >      free(p_credential->psz_dialog_password);
> >  }
> > 
> > +static int
> > +credential_get_next_order(vlc_credential *p_credential, vlc_object_t
> > *p_parent) +{
> > +    (void) p_parent;
> > +
> > +    /* DIALOG is the last way to fetch credential, use it until credentials
> > are +     * valid or until the user cancel it */
> > +    if (p_credential->i_get_order == GET_FROM_DIALOG)
> > +        return p_credential->i_get_order;
> > +
> > +    return ++p_credential->i_get_order;
> > +}
> > +
> >  #undef vlc_credential_get
> >  bool
> >  vlc_credential_get(vlc_credential *p_credential, vlc_object_t *p_parent,
> > @@ -401,8 +414,10 @@ vlc_credential_get(vlc_credential *p_credential,
> > vlc_object_t *p_parent, * Finally, fetch credential from the dialog (if
> > any). This last will be * repeated until user cancel the dialog. */
> > 
> > -        switch (p_credential->i_get_order)
> > +        switch (credential_get_next_order(p_credential, p_parent))
> >          {
> > +        case GET_FROM_INIT:
> > +            return false;
> >          case GET_FROM_URL:
> >              p_credential->psz_username = p_url->psz_username;
> >              p_credential->psz_password = p_url->psz_password;
> > @@ -412,7 +427,6 @@ vlc_credential_get(vlc_credential *p_credential,
> > vlc_object_t *p_parent,
> > 
> >              if (p_url->psz_username && protocol_is_smb(p_url))
> >                  smb_split_domain(p_credential);
> > -            p_credential->i_get_order++;
> >              break;
> > 
> >          case GET_FROM_OPTION:
> > @@ -432,8 +446,6 @@ vlc_credential_get(vlc_credential *p_credential,
> > vlc_object_t *p_parent, p_credential->psz_username =
> > p_credential->psz_var_username; if (p_credential->psz_var_password)
> >                  p_credential->psz_password =
> > p_credential->psz_var_password; -
> > -            p_credential->i_get_order++;
> >              break;
> > 
> >          case GET_FROM_MEMORY_KEYSTORE:
> > @@ -441,7 +453,6 @@ vlc_credential_get(vlc_credential *p_credential,
> > vlc_object_t *p_parent, vlc_keystore *p_keystore =
> > get_memory_keystore(p_parent); if (p_keystore != NULL)
> >                  credential_find_keystore(p_credential, p_keystore);
> > -            p_credential->i_get_order++;
> >              break;
> >          }
> > 
> > @@ -453,11 +464,8 @@ vlc_credential_get(vlc_credential *p_credential,
> > vlc_object_t *p_parent, p_credential->p_keystore =
> > vlc_keystore_create(p_parent); if (p_credential->p_keystore != NULL)
> >                  credential_find_keystore(p_credential,
> > p_credential->p_keystore); -
> > -            p_credential->i_get_order++;
> >              break;
> > 
> > -        default:
> >          case GET_FROM_DIALOG:
> >              if (!psz_dialog_title || !psz_dialog_fmt)
> >                  return false;
> > @@ -496,6 +504,9 @@ vlc_credential_get(vlc_credential *p_credential,
> > vlc_object_t *p_parent, smb_split_domain(p_credential);
> > 
> >              break;
> > +
> > +        default:
> > +            vlc_assert_unreachable();
> >          }
> >      }
> >      return is_credential_valid(p_credential);
> 
> 
> -- 
> レミ・デニ-クールモン
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel