[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: svn commit: r997026 - /subversion/trunk/subversion/mod_dav_svn/repos.c

From: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: Wed, 10 Nov 2010 06:58:12 -0600

On Wed, Nov 10, 2010 at 5:53 AM, Julian Foad <julian.foad_at_wandisco.com> wrote:
> C. Michael Pilato wrote:
>> On 11/09/2010 03:00 PM, Stefan Sperling wrote:
>> >> @@ -4046,7 +4050,16 @@ do_walk(walker_ctx_t *ctx, int depth)
>> >>        /* authorize access to this resource, if applicable */
>> >>        if (params->walk_type & DAV_WALKTYPE_AUTH)
>> >>          {
>> >> -          /* ### how/what to do? */
>> >> +          const char *repos_relpath =
>> >> +            apr_pstrcat(iterpool,
>> >> +                        apr_pstrmemdup(iterpool,
>> >> +                                       ctx->repos_path->data,
>> >> +                                       ctx->repos_path->len),
>> >> +                        key, NULL);
>> >
>> > Here, key is const void*. Cast it const char* for clarity?
>>
>> In the past, we've frowned on all unnecessary casts.  I was merely adhering
>> to a decade's worth of Subversion code tradition.  :-)
>
> Avoid casts.  An old traditional style is
>
>  void *key;
>  const char *propname /* or whatever the key represents */;
>
>  apr_hash_this(hi, &key, ...);
>  propname = key;
>
> and a neater, more recent option is
>
>  const char *propname = svn_apr_hash_index_key(hi);

Minor nit: it's actually svn__apr_hash_index_key(hi) (note the double
underscore).

-Hyrum

>
>
> - Julian
>
>
>> > Are we guaranteed that repos_path->data always ends with a slash,
>> > or that key always starts with one?
>> > Why not use svn_{dirent,uri,relpath}_join() or similar?
>>
>> "key" is guaranteed to be a single path component (relative).
>>
>> But outside the diff context is this code:
>>
>>   if (ctx->repos_path->data[ctx->repos_path->len - 1] != '/')
>>     svn_stringbuf_appendcstr(ctx->repos_path, "/");
>>
>> ctx->repos_path is a stringbuf that's constantly being telescoped and
>> de-telescoped.  It's 'data' component cannot be used independently of its
>> 'len' component (if you want the expected results, at least).
>>
>
>
>
Received on 2010-11-10 13:58:55 CET

This is an archived mail posted to the Subversion Dev mailing list.