Garrett Rooney <rooneg@electricjellyfish.net> writes:
> > > + 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?
In this case I don't think so, they simply get passed to functions
that take a pointer.
>
> > > +
> > > +#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.
Yes, I see. This looks like a case of the programmer trying to
outguess the compiler. I would use a variable and let the optimiser
decide whether to remove it or not.
>
> > 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.
I see. Sorry, my mistake.
--
Philip
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 21 14:36:57 2006