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

Re: svn commit: rev 4621 - in trunk/subversion: include libsvn_client clients/cmdline svnversion

From: <gstein_at_lyra.org>
Date: 2003-01-30 01:53:10 CET

On Mon, Jan 27, 2003 at 06:13:01PM -0600, rooneg@tigris.org wrote:
...
 +++ trunk/subversion/libsvn_client/switch.c Mon Jan 27 18:12:38 2003
...
 @@ -133,7 +134,9 @@
    /* Get the RA vtable that matches working copy's current URL. */
    SVN_ERR (svn_ra_init_ra_libs (ra_baton, pool));
    SVN_ERR (svn_ra_get_ra_library (ra_lib, ra_baton, URL, pool));
 -
 +
 + SVN_ERR (svn_client_ctx_get_auth_baton (ctx, auth_baton));

Eek. This code is *inside* libsvn_client. Our typical pattern is that a
structure is opaque outside the library, but completely visible inside the
library. Thus, we don't need to use getters/setters within the library. The
code can simply refer to ctx-auth_baton.

...
    else if ((src_is_url) (! dst_is_url))
 - SVN_ERR (repos_to_wc_copy (src_path, src_revision,
 - dst_path, optional_adm_access, auth_baton,
 - notify_func, notify_baton,
 - pool));
 + {
 + svn_client_auth_baton_t *auth_baton;
 +
 + SVN_ERR (svn_client_ctx_get_auth_baton (ctx, auth_baton));
  
 + SVN_ERR (repos_to_wc_copy (src_path, src_revision,
 + dst_path, optional_adm_access, auth_baton,
 + notify_func, notify_baton,
 + pool));
 + }

>From a simple line to a big block. Keeping the structure visible within the
library would avoid this kind of stuff.

...
 +++ trunk/subversion/libsvn_client/context.c Mon Jan 27 18:12:46 2003
...
 +svn_error_t *
 +svn_client_ctx_get_auth_baton (svn_client_ctx_t *ctx,
 + svn_client_auth_baton_t **auth_baton)
 +{
 + *auth_baton = ctx-auth_baton;
 +
 + if (! *auth_baton)
 + return svn_error_create (SVN_ERR_CLIENT_CTX_NOT_FOUND, NULL,
 + no authentication baton found);

Why the whole error thing? Just return NULL. It could be quite a valid
approach for the caller to want to check if auth stuff has been installed or
not. They shouldn't have to deal with error returns.

Basically, it seems like there is too much engineering around what could
be a simple structure.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@subversion.tigris.org
For additional commands, e-mail: dev-help@subversion.tigris.org
Received on Sat Oct 14 02:24:49 2006

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

This site is subject to the Apache Privacy Policy and the Apache Public Forum Archive Policy.