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 <[email protected]>
CollabNet <> www.collab.net <> Distributed Development On Demand
signature.asc
Description: OpenPGP digital signature
