On 11/09/2010 03:00 PM, Stefan Sperling wrote:
> Hi Mike,
>
> I'm reviewing this for 1.6.14. I have two questions (see below).
>
>> * subversion/mod_dav_svn/repos.c
>> (do_walk): Replace a decade-old TODO about checking authorization on
>> a directory's children with, you know, code that checks
>> authorization on a directory's children.
>>
>> Modified:
>> subversion/trunk/subversion/mod_dav_svn/repos.c
>>
>> Modified: subversion/trunk/subversion/mod_dav_svn/repos.c
>> URL: http://svn.apache.org/viewvc/subversion/trunk/subversion/mod_dav_svn/repos.c?rev=997026&r1=997025&r2=997026&view=diff
>> ==============================================================================
>> --- subversion/trunk/subversion/mod_dav_svn/repos.c (original)
>> +++ subversion/trunk/subversion/mod_dav_svn/repos.c Tue Sep 14 18:08:59 2010
[...]
>> @@ -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. :-)
> 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).
--
C. Michael Pilato <cmpilato_at_collab.net>
CollabNet <> www.collab.net <> Distributed Development On Demand
Received on 2010-11-09 21:33:10 CET