[svn.haxx.se] · SVN Dev · SVN Users · SVN Org · TSVN Dev · TSVN Users · Subclipse Dev · Subclipse Users · this month's index

Re: [PATCH] add --limit argument to svn log

From: Garrett Rooney <rooneg_at_electricjellyfish.net>
Date: 2004-09-25 15:34:15 CEST

Greg Hudson wrote:
> The biggest problem I see:
>
> - SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)(?r)(?r)bb)", start, end,
> + SVN_ERR(svn_ra_svn_write_tuple(conn, pool, "!)(?r)(?r)bbr)", start, end,
>
> I know I left a little gap in the ra_svn infrastructure by not allowing
> 'n's in the optional part of a tuple, but you don't get to gloss over
> that by pretending that your limit argument is a revision. That's a big
> giant -1.
>
> I suggest introducing SVN_RA_SVN_UNSPECIFIED_NUMBER, #defined to
> ~((unsigned apr_uint64_t) 0), and using that as the distinguished value
> for an unspecified optional number.

That makes sense. I agree, the current way of doing it (passing a
revision) is a hack.

> Other issues:
>
> In libsvn_repos/log.c, I would add a counter to the for loop rather than
> adjusting start or end. It's more straightforward, and I'm a little
> concerned about the correctness of your code in light of the continue
> clause in the loop body. Since the loop conditions are already
> complicated, the counter check might be best written as an "if (++count
>>= limit) break;" (or whatever) in the loop body, after the continue
>
> clause has been given a chance to fire.

I'll take a look.

> I think the limit argument should just be an int, not a "long int". Our
> code base only very rarely uses the "long" type. The only real
> difference between "int" and "long" is that long is guaranteed to be 32
> bits, and if ints are 16 bits (you have to go back pretty far to find an
> environment like that), we're already very sunk.

It was a long int so I could use the code for passing around revisions
in ra_svn ;-) That can certainly go away once that hack is removed.

> In three places you wrote "compatability", which is a misspelling of
> "compatibility".

Oops. Good catch. I'm too used to working with a copy editor who
catches those sort of things for me ;-)

Thanks for the review!

-garrett

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Sep 25 15:34:12 2004

This is an archived mail posted to the Subversion Dev mailing list.