Re: svn commit: r15993 - trunk/subversion/svnserve

[Greg Hudson <gh...@MIT.EDU>]

On Tue, 2005-08-30 at 11:08 -0500, kfogel@tigris.org wrote:
>   (must_have_access): Move pool to end of parameter list, as discussed
>   in review of r15982.  All callers changed.

I believe you just took a single function, which people happened to
notice because it was touched in r15982, and made it inconsistent with
the rest of serve.c.  Didn't you notice you were doing that?


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r15993 - trunk/subversion/svnserve

[kf...@collab.net]

David James <ja...@gmail.com> writes:
> There doesn't seem to be a consistent pool parameter convention in serve.c.

No, I think there is.

Functions which take a connection as the first argument, take a pool
as their second argument.  Everyone else takes (or should take) a pool
as the last argument, if they take a pool at all.

> (NOTE: Since these are all private functions, the SWIG bindings don't
> care about them. But, it's still a good idea to use the pool as the
> last argument in new functions, in case we want to make a build a
> public API function with the same interface.)
> 
> Functions with pool as last argument:
> - authz_check_access
> - authz_check_access_cb
> - authz_commit_cb
> - must_have_access
> - get_props
> - add_lock_tokens
> - unlock_paths

Well, add_lock_tokens() is violating the pattern, because it takes a
connection as its first argument, but everyone else is consistent.

> Functions with pool as first argument:
> - lookup_access

Yeah, this is just wrong.

> Functions with pool as second argument:
> - send_mechs
> - create_fs_access
> - auth
> - auth_request
> - trivial_auth_request
> - set_path
> - delete_path
> - link_path
> - finish_report
> - abort_report
> - accept_report
> - write_proplist
> - write_prop_diffs
> - write_lock
> - get_latest_rev
> - get_dated_rev
> - change_rev_prop
> - rev_proplist
> - rev_prop
> - commit
> - get_file

I haven't checked absolutely all of them, but I believe these are all
of the func(conn, pool, ...) flavor.

-Karl

-- 
www.collab.net  <>  CollabNet  |  Distributed Development On Demand


---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r15993 - trunk/subversion/svnserve

[David James <ja...@gmail.com>]

On 8/30/05, Greg Hudson <gh...@mit.edu> wrote:
> On Tue, 2005-08-30 at 11:08 -0500, kfogel@tigris.org wrote:
> >   (must_have_access): Move pool to end of parameter list, as discussed
> >   in review of r15982.  All callers changed.
> I believe you just took a single function, which people happened to
> notice because it was touched in r15982, and made it inconsistent with
> the rest of serve.c.  Didn't you notice you were doing that?
There doesn't seem to be a consistent pool parameter convention in serve.c.

(NOTE: Since these are all private functions, the SWIG bindings don't
care about them. But, it's still a good idea to use the pool as the
last argument in new functions, in case we want to make a build a
public API function with the same interface.)

Functions with pool as last argument:
- authz_check_access
- authz_check_access_cb
- authz_commit_cb
- must_have_access
- get_props
- add_lock_tokens
- unlock_paths

Functions with pool as first argument:
- lookup_access

Functions with pool as second argument:
- send_mechs
- create_fs_access
- auth
- auth_request
- trivial_auth_request
- set_path
- delete_path
- link_path
- finish_report
- abort_report
- accept_report
- write_proplist
- write_prop_diffs
- write_lock
- get_latest_rev
- get_dated_rev
- change_rev_prop
- rev_proplist
- rev_prop
- commit
- get_file

-- 
David James -- http://www.cs.toronto.edu/~james

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org

Re: svn commit: r15993 - trunk/subversion/svnserve

[kf...@collab.net]

Greg Hudson <gh...@MIT.EDU> writes:
> On Tue, 2005-08-30 at 11:08 -0500, kfogel@tigris.org wrote:
> >   (must_have_access): Move pool to end of parameter list, as discussed
> >   in review of r15982.  All callers changed.
> 
> I believe you just took a single function, which people happened to
> notice because it was touched in r15982, and made it inconsistent with
> the rest of serve.c.  Didn't you notice you were doing that?

I made a quick check for consistency, but it just so happens that the
first few functions in serve.c put their pools at the end, so I
assumed it was must_have_access() which was inconsistent.  Of course,
those first few function are not of the func(conn, pool, ...) variety,
that's why the pool doesn't come second for them!

Reverted in r16004, thanks for noticing.

-Karl

-- 
www.collab.net  <>  CollabNet  |  Distributed Development On Demand

---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org