You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@subversion.apache.org by Daniel Shahaf <d....@daniel.shahaf.name> on 2012/09/01 23:01:45 UTC

Re: svn commit: r1375052 - /subversion/trunk/subversion/libsvn_subr/auth.c

Backport review:

cmpilato@apache.org wrote on Mon, Aug 20, 2012 at 15:19:06 -0000:
> Author: cmpilato
> Date: Mon Aug 20 15:19:06 2012
> New Revision: 1375052
> 
> URL: http://svn.apache.org/viewvc?rev=1375052&view=rev
> Log:
> Minor code simplifications.  No logical changes.
> 
> * subversion/libsvn_subr/auth.c
>   (SVN__MAYBE_ADD_PROVIDER): New macro.
>   (svn_auth_get_platform_specific_client_providers): Avoid logic
>     branch by recognizing that svn_config_get() returns the default
>     value when the passed-in config object is NULL.  Also, replace a
>     bunch of "if-provider-isn't-NULL-then-add-the-array" logic with
>     SVN__MAYBE_ADD_PROVIDER() macro calls.
> 
> Modified:
>     subversion/trunk/subversion/libsvn_subr/auth.c
> 
> Modified: subversion/trunk/subversion/libsvn_subr/auth.c
> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/libsvn_subr/auth.c?rev=1375052&r1=1375051&r2=1375052&view=diff
> ==============================================================================
> --- subversion/trunk/subversion/libsvn_subr/auth.c (original)
> +++ subversion/trunk/subversion/libsvn_subr/auth.c Mon Aug 20 15:19:06 2012
> @@ -523,34 +523,29 @@ svn_auth_get_platform_specific_client_pr
>    apr_array_header_t *password_stores;
>    int i;
>  
> +#define SVN__MAYBE_ADD_PROVIDER(list, p) \
> +  { if (p) APR_ARRAY_PUSH(list, svn_auth_provider_object_t *) = p; }
> +

Parentheses around uses of macro parameters?

  +  { if ((p)) APR_ARRAY_PUSH((list), svn_auth_provider_object_t *) = (p); }

> -      return svn_error_createf(SVN_ERR_BAD_CONFIG_VALUE, NULL,
> -                               _("Invalid config: unknown password store "
> -                                 "'%s'"),
> -                               password_store);

I think this is the only operational change in this revision. It would
have been easier to review this revision if it consisted of a single
hunk removing the above four lines.

>      }
>  
>    return SVN_NO_ERROR;
> 
>