Re: [PATCH] fix for issue #591

On Sun, Jan 20, 2002 at 06:06:42PM +0000, Philip Martin wrote:
 
> Subversion style is to put a space between the function and the
> opening parenthesis [this affects most of the other function calls in
> this patch].

as sander said, that is the style in this file, but not in all of
them. my mistake in this spot, but at least one of the files i
touched did not have the spaces.

> > + apr_hash_this(hi, (const void **)&key, NULL, (void **)&val);
>
> Existing code generally avoids the casts and declares
>
> const void *key;
> apr_ssize_t klen;
> void *val;
> apr_hash_this (hi, &key, &klen, &val);

wouldn't i just have to cast it later when i use them?

> > +
> > +#define NSLEN (sizeof(SVN_PROP_CUSTOM_PREFIX) - 1)
> > + if (strncmp(key, SVN_PROP_CUSTOM_PREFIX, NSLEN) == 0)
> > + apr_hash_set(*props, &key[NSLEN], APR_HASH_KEY_STRING,
> > + svn_string_create(val, ras->pool));
> > +#undef NSLEN
>
> Why the preprocessor abuse? Existing code uses a variable such as
> apr_size_t custom_prefix_len = sizeof (SVN_PROP_CUSTOM_PREFIX) - 1;

existing code in this file does exactly the same thing i did with the
#define. i thought it was a little odd as well though, but figured i
should try to be consistent.

> However, I worry about the use of strncmp. As far as I can see a user
> can create a property called "custom:xxx" which will erroneously
> compare equal if you use the above test. Now perhaps that should not
> be allowed, but a strcmp, or a klen==custom_prefix_len test, would
> avoid the problem.

the strncmp is comparing against the svn:custom: part that we prefix
our properties with when sending them over dav. if a user defined a
custom property "custom:xxx", it would be sent as "svn:custom:custom:xxx",
so i think it will still work.

this is the way that properties are handled in the rest of the file...
i'm not certain if it's the best way, but i figured i'd post it and
let gstein (the author of the surrounding code) comment.

-garrett

-- 
garrett rooney                     Unix was not designed to stop you from 
rooneg@electricjellyfish.net       doing stupid things, because that would  
http://electricjellyfish.net/      stop you from doing clever things.
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org