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

[Rémi Denis-Courmont]

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.

> 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/