Greg Stein <gstein@lyra.org> writes:
> With new functions, can we get them to use "const svn_string_t *" ??
Actually, these functions have been const-char-*, const-svn_string_t-*, and
const-svn_stringbuf_t-*. I decided in the end to keep them as
stringbuf because all other path functions are so, but with full
intent to change them the minute their use cases start to lean to
another thing.
So, here's the deal.
I'd love for them to use svn_string_t, but I was trying to find a
function that would create an svn_string_t from an existing char *
that *didn't* duplicate the allocation. But then you'd have a
structure allocated in one pool and its data allocated who-knows-where.
Perhaps its callers should just instantiate an svn_string_t on the
stack and hand-populate its members?
> >...
> > +static svn_boolean_t
> > +char_is_uri_safe (const unsigned char c)
>
> No need for unsigned, is there? And const is kinda weird for a pass-by-value
> argument :-) (certainly doesn't hurt tho)
Oh yeah. const on pass-by-value was dumb. As for the unsigned,
aren't we encoding a hex value between 0 and 255, not between -128 and
127? But as you pointed out later, str->data is probably signed anyway.
> > +{
> > + unsigned char other[18] = "/:.-_!~'()@=+$,&*"; /* sorted by estimated use? */
>
> If you do:
>
> static const other[] = "...";
>
> Then you won't have to copy the contents on function entry(!!), and the size
> will be automatically computed.
Mmmm...yes.
> >...
> > + /* Is this a supported non-alphanumeric character? */
> > + for (i = 0; i < sizeof (other); i++)
>
> That sizeof() is going to include the null-term because of the above
> definitions.
>
> However, it would probably be faster to do:
>
> if (strchr("...", c) != NULL)
> return TRUE;
Okey dokey.
---------------------------------------------------------------------
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:45 2006