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);
- 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).
>
