You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@subversion.apache.org by Greg Hudson <gh...@MIT.EDU> on 2002/12/04 06:59:11 UTC

Re: svn commit: rev 3983 - in trunk: . subversion/include subversion/libsvn_ra_svn subversion/svnserve

Thanks.  A few small complaints:

On Wed, 2002-12-04 at 01:09, brane@tigris.org wrote:
>   -- "log" happens to be a standard C function; for consistency all
>      the svnserve vtable function names got a prefix.

I had a convention for this, which was to add "_cmd" to just the symbols
which had conflicts.  (switch_cmd was the only one so far.)  Calling
them ra_svn_foo means the server functions have the same names as the
client functions, which can be inconvenient when setting breakpoints.

> -static svn_error_t *get_latest_rev(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> -                                   apr_array_header_t *params, void *baton)
> +static svn_error_t *
> +ra_svn_get_latest_rev(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> +                      apr_array_header_t *params, void *baton)

Unless we have a tree-wide style policy on this point, I'd prefer if you
wouldn't reformat these function declarations.


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

Re: svn commit: rev 3983 - in trunk: . subversion/include subversion/libsvn_ra_svn subversion/svnserve

Posted by Branko Čibej <br...@xbc.nu>.
Greg Stein wrote:

>On Fri, Dec 06, 2002 at 08:51:07AM -0500, Greg Hudson wrote:
>
>>These also happen to be static functions.
>>
>>Of course, I could bring in "bad" header files.  But I'd find out about
>>that pretty quickly.
>>    
>>
>
>You'd only find out on the platform that you work with. Meanwhile, the evil
>demons are plotting your destruction when you're built on BeOS...
>
Yup, the "log" thing didn't show up on (presumably) Linux, but did show
up on Windows.

I've reverted my renames for now, but I'd strongly suggest to put some
sort of prefix in anyway.

-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


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

Re: svn commit: rev 3983 - in trunk: . subversion/include subversion/libsvn_ra_svn subversion/svnserve

Posted by Greg Stein <gs...@lyra.org>.
On Fri, Dec 06, 2002 at 08:51:07AM -0500, Greg Hudson wrote:
> On Thu, 2002-12-05 at 17:55, Greg Stein wrote:
> > On Wed, Dec 04, 2002 at 10:35:04PM -0500, Greg Hudson wrote:
> > >...
> > > My reasoning is that prefixing is for libraries; svnserve is an
> > > application.  (libsvn_ra_svn does use an ra_svn prefix on its vtable
> > > functions.)
> > 
> > That still isn't all that safe. If you link in "bad" libraries, then your
> > app could have conflicts. Now, if you only link in "good" libraries, then
> > your app can name things however it wants...
> 
> These also happen to be static functions.
> 
> Of course, I could bring in "bad" header files.  But I'd find out about
> that pretty quickly.

You'd only find out on the platform that you work with. Meanwhile, the evil
demons are plotting your destruction when you're built on BeOS...

:-)

It's all a matter of how paranoid you want to be...

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

Re: svn commit: rev 3983 - in trunk: . subversion/include subversion/libsvn_ra_svn subversion/svnserve

Posted by Greg Hudson <gh...@MIT.EDU>.
On Thu, 2002-12-05 at 17:55, Greg Stein wrote:
> On Wed, Dec 04, 2002 at 10:35:04PM -0500, Greg Hudson wrote:
> >...
> > My reasoning is that prefixing is for libraries; svnserve is an
> > application.  (libsvn_ra_svn does use an ra_svn prefix on its vtable
> > functions.)
> 
> That still isn't all that safe. If you link in "bad" libraries, then your
> app could have conflicts. Now, if you only link in "good" libraries, then
> your app can name things however it wants...

These also happen to be static functions.

Of course, I could bring in "bad" header files.  But I'd find out about
that pretty quickly.


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

Re: svn commit: rev 3983 - in trunk: . subversion/include subversion/libsvn_ra_svn subversion/svnserve

Posted by Greg Stein <gs...@lyra.org>.
On Wed, Dec 04, 2002 at 10:35:04PM -0500, Greg Hudson wrote:
>...
> My reasoning is that prefixing is for libraries; svnserve is an
> application.  (libsvn_ra_svn does use an ra_svn prefix on its vtable
> functions.)

That still isn't all that safe. If you link in "bad" libraries, then your
app could have conflicts. Now, if you only link in "good" libraries, then
your app can name things however it wants...

"Are you a good witch? or a bad witch?"

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

Re: svn commit: rev 3983 - in trunk: . subversion/include subversion/libsvn_ra_svn subversion/svnserve

Posted by Branko Čibej <br...@xbc.nu>.
Greg Hudson wrote:

>My reasoning is that prefixing is for libraries; svnserve is an
>application.  (libsvn_ra_svn does use an ra_svn prefix on its vtable
>functions.)
>
Right. change that back, except the log -> log:_cmd rename.

>>>Unless we have a tree-wide style policy on this point, I'd prefer if
>>>you wouldn't reformat these function declarations.
>>>      
>>>
>>As a matter of fact, we _do_ have a style policy, it just so happens that both
>>Gregs tend to ignore it. :-)
>>    
>>
>
>Huh.  Okay, I will go back and reformat all the other function
>declarations to match.  But I'm going to punt the prefix on the svnserve
>command handlers.
>  
>
No, don't bother reformatting all the declarations. We're not
_completely_ consistent, anyway; mod_dav_svn is a case in point. Like I
said, I reformatted those decls to make them fit into 80 columns (after
the rename), not because of some philosophical reason.


-- 
Brane Čibej   <br...@xbc.nu>   http://www.xbc.nu/brane/


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

Re: svn commit: rev 3983 - in trunk: . subversion/include subversion/libsvn_ra_svn subversion/svnserve

Posted by Greg Hudson <gh...@MIT.EDU>.
On Wed, 2002-12-04 at 12:00, brane@xbc.nu wrote:
> > I had a convention for this, which was to add "_cmd" to just the symbols
> > which had conflicts.  (switch_cmd was the only one so far.)  Calling
> > them ra_svn_foo means the server functions have the same names as the
> > client functions, which can be inconvenient when setting breakpoints.
> 
> Ah, O.K. The other RA libs use a common prefix on the vtable functions, so in
> fact to be consistent the prefix should be svn_ra_svn__ (even on static
> functions). Personally I'd rather see those names prefixed, so that the purpose
> of the functions is obvious from the names. I'd also prefer to have the function
> names (after the prefix) the same as in ra_dav and ra_local.

My reasoning is that prefixing is for libraries; svnserve is an
application.  (libsvn_ra_svn does use an ra_svn prefix on its vtable
functions.)

> > Unless we have a tree-wide style policy on this point, I'd prefer if
> > you wouldn't reformat these function declarations.

> As a matter of fact, we _do_ have a style policy, it just so happens that both
> Gregs tend to ignore it. :-)

Huh.  Okay, I will go back and reformat all the other function
declarations to match.  But I'm going to punt the prefix on the svnserve
command handlers.


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

Re: svn commit: rev 3983 - in trunk: . subversion/include subversion/libsvn_ra_svn subversion/svnserve

Posted by br...@xbc.nu.
Quoting Greg Hudson <gh...@MIT.EDU>:

> Thanks.  A few small complaints:
> 
> On Wed, 2002-12-04 at 01:09, brane@tigris.org wrote:
> >   -- "log" happens to be a standard C function; for consistency all
> >      the svnserve vtable function names got a prefix.
> 
> I had a convention for this, which was to add "_cmd" to just the symbols
> which had conflicts.  (switch_cmd was the only one so far.)  Calling
> them ra_svn_foo means the server functions have the same names as the
> client functions, which can be inconvenient when setting breakpoints.

Ah, O.K. The other RA libs use a common prefix on the vtable functions, so in
fact to be consistent the prefix should be svn_ra_svn__ (even on static
functions). Personally I'd rather see those names prefixed, so that the purpose
of the functions is obvious from the names. I'd also prefer to have the function
names (after the prefix) the same as in ra_dav and ra_local.

 
> > -static svn_error_t *get_latest_rev(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> > -                                   apr_array_header_t *params, void *baton)
> > +static svn_error_t *
> > +ra_svn_get_latest_rev(svn_ra_svn_conn_t *conn, apr_pool_t *pool,
> > +                      apr_array_header_t *params, void *baton)
> 
> Unless we have a tree-wide style policy on this point, I'd prefer if
> you wouldn't reformat these function declarations.

As a matter of fact, we _do_ have a style policy, it just so happens that both
Gregs tend to ignore it. :-)
Seruously: I reformatted them because the names got longer and the declarations
wouldn't fit in 80 columns any more.

    Brane

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