You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Branko Čibej <br...@wandisco.com> on 2014/10/28 17:26:40 UTC

Re: svn commit: r1634906 - in /subversion/branches/svn-auth-x509/subversion: include/svn_x509.h libsvn_subr/x509.h libsvn_subr/x509info.c libsvn_subr/x509parse.c svn/auth-cmd.c tests/libsvn_subr/x509-test.c

On 28.10.2014 17:17, breser@apache.org wrote:
> Author: breser
> Date: Tue Oct 28 16:17:47 2014
> New Revision: 1634906

[...]


> +static apr_array_header_t *
> +deep_copy_array(apr_array_header_t *s, apr_pool_t *result_pool)
> +{
> +      int i;
> +      apr_array_header_t *d;
> +
> +      if (!s)
> +        return NULL;
> +
> +      d = apr_array_copy(result_pool, s);
> +
> +      /* Make a deep copy of the strings in the array. */
> +      for (i = 0; i < s->nelts; ++i)
> +        APR_ARRAY_IDX(d, i, const char *) =
> +          apr_pstrdup(result_pool,
> +                      APR_ARRAY_IDX(s, i, const char *));
> +
> +      return d;
> +}

Indentation is wrong. And I suggest using svn_iter_apr_array here.


> +
> +static apr_hash_t *deep_copy_hash(apr_hash_t *s,
> +                                  apr_pool_t *scratch_pool, 
> +                                  apr_pool_t *result_pool)
> +{
> +  apr_hash_t *d;
> +  apr_hash_index_t *i;
> +
> +  if (!s)
> +    return NULL;
> +
> +  d = apr_hash_make(result_pool);
> +  i = apr_hash_first(scratch_pool, s);
> +
> +  /* Make a deep copy of the hash keys and values. */
> +  while (i)
> +    {
> +      const void *key;
> +      void *val;
> +      apr_ssize_t klen;
> +
> +      apr_hash_this(i, &key, &klen, &val);
> +      apr_hash_set(d, apr_pstrndup(result_pool, key, klen), klen,
> +                   apr_pstrdup(result_pool, val));
> +      i = apr_hash_next(i);
> +    }
> +
> +  return d;
> +}

... and svn_iter_apr_hash here.


-- Brane

Re: svn commit: r1634906 - in /subversion/branches/svn-auth-x509/subversion: include/svn_x509.h libsvn_subr/x509.h libsvn_subr/x509info.c libsvn_subr/x509parse.c svn/auth-cmd.c tests/libsvn_subr/x509-test.c

Posted by Branko Čibej <br...@wandisco.com>.
On 30.10.2014 02:00, Ben Reser wrote:
> On 10/28/14 9:26 AM, Branko Čibej wrote:
>> Indentation is wrong.
> Whoops, somehow my code style setting got disabled when I wrote this and didn't
> notice it.  Fixed in r1635359.
>
>> And I suggest using svn_iter_apr_array here.
>> ... and svn_iter_apr_hash here.
> Unless it's a big deal to you I'm not inclined to use those here.  They feel
> very clunky for this use.  For one thing I need to add another function for the
> callback that have only one line.  For another I have to pass the pool into the
> callback baton since they both pass only a scratch pool.

I agree they're somewhat cluncky (but that's fixable). However, at least
svn_iter_apr_hash is faster (with a new-enough APR) than the most
obvious hash iteration loop.

C needs lambdas and closures ...

-- Brane

Re: svn commit: r1634906 - in /subversion/branches/svn-auth-x509/subversion: include/svn_x509.h libsvn_subr/x509.h libsvn_subr/x509info.c libsvn_subr/x509parse.c svn/auth-cmd.c tests/libsvn_subr/x509-test.c

Posted by Ben Reser <be...@reser.org>.
On 10/28/14 9:26 AM, Branko Čibej wrote:
> Indentation is wrong.

Whoops, somehow my code style setting got disabled when I wrote this and didn't
notice it.  Fixed in r1635359.

> And I suggest using svn_iter_apr_array here.
> ... and svn_iter_apr_hash here.

Unless it's a big deal to you I'm not inclined to use those here.  They feel
very clunky for this use.  For one thing I need to add another function for the
callback that have only one line.  For another I have to pass the pool into the
callback baton since they both pass only a scratch pool.