Re: svn commit: r15982 - in trunk: notes subversion/include subversion/libsvn_repos

kfogel@collab.net wrote:
> As I think was discussed on the list, the comment above those three
> doesn't quite apply to all of them. I can't remember if we decided
> that that shouldn't be addressed in this commit, though; just wanted
> to make sure it didn't get forgotten.
>
> (If you don't plan/want to do anything about it, that's fine, just let
> me know so I can :-) ).

Not sure if I replied to that, but I feel this is a separate problem, to
be corrected separately.

Right now I'm in the midst of fixing issue #2388. If you care enough to
fix the defines before I finish, then go ahead. Otherwise, it's on my
todo.svn .

> The parameter's name is actually B, not BATON :-).
>
> (I don't have a problem with calling it 'b', but if it's going to make
> the doc string more awkward to write, then I'd say change the
> variable's name to 'baton'.)

There is a consistency issue in the whole of svnserve here I think. On
one hand there is the name used for a void* (usually 'baton'), and on
the other the name used for the pointer to the right type (usually 'b'
or 'sb'. I'll comb back through serve.c and make all this consistent,
probably making everything to void *baton and struct ... *b (with
apologies to Branko's itching froodleprog).

> The code still looks good the second time around. That's comforting! :-)

Like I said, that bit is squishy, but in a firm enough sense that it
will hold the fort until I get back to it (currently number 3 on my
todo.svn).

> It's unusual to put POOL anywhere other than the end of the parameter
> list; was there a reason? (Sorry, didn't notice last time around.)

Historical: must_have_write_access(), which this function evolved from,
had the parameters in that order. At the time I was more concerned
about how I was going to structure the code to accomodate both authz and
blanket access control, and so forgot about it.

But, it's an internal function, and can therefore be shoved around and
reprototyped easily.

Once I'm done killing #2388, I'll commit the fixes for all this.

Thanks for the (thorough as ever!) review.
- Dave.

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Tue Aug 30 07:36:43 2005