Another type issue.

[cm...@collab.net]

Karl and I are chatting about the FS function arguments of type 'char
*', specifically about how those aren't svn_string_t's.  Is there any
compelling reason why these should *not* be svn_string_t's?  

Karl sez that the svn_path utilities work best with svn_string_t's as
opposed to regular ol' "char-stars", yet we have a pretty strong
suspicion that those interfaces using the FS code would rather speak
native types as opposed to svn_string_t's.  So, somewhere, someone has
to do conversions between to two types.

The conversion from svn_string_t to char * is trivial (string->data),
and the reverse isn't altogether painful either.  But there are
differences between the two (read: "This is not another svn_boolean_t
discussion."), so some background and debate are encouraged.

Re: Another type issue.

[Greg Stein <gs...@lyra.org>]

On Mon, Feb 26, 2001 at 05:01:51PM -0500, Greg Hudson wrote:
> We should use svn_string_t whenever:
> 
> 	* We are passing a mutable string, or
> 	* We need to represent binary data (including 0 bytes)
> 
> We should use char * whenever the above aren't true, i.e. when we're
> passing an immutable string which can only contain text data.
> 
> ---
> That's my position.  Some rationale:
> 
> We could use svn_string_t everywhere.  It would be kind of a pain in
> the butt (because we'd have to convert every time we wanted to use a
> system call or apr string-handling routine, and because we'd have to
> worry about memory allocation even when we were just passing a
> literal), but it would work.  But we've rejected that approach.
> 
> Frankly, I think it's a tiny bit unclean to pass an svn_string_t * for
> an immutable string when we are representing binary data.  But blame C
> for using a half-assed string type for that gaffe.  A "const"
> qualifier will make it clear that the called function isn't allowed to
> modify the string.

I'm with Greg on this one. There are a couple lines in commit.c:

  /* ### damn this is annoying to have to create a string */
  svn_string_t * propname = svn_string_create(SVN_RA_DAV__LP_ACTIVITY_URL,
                                              cc->ras->pool);

I needed to pass a constant property name to change_file_prop (it will be
set_wc_prop in the future), but the API required an svn_string_t. Bleck.


Re: Jim's comment on path handling: we have much of that in Apache right
now, and some stuff is already on the table for addition to APR. We can move
the rest into APR. IOW, yes: we'll have quite fine path handling for char*
style strings.

Cheers,
-g

-- 
Greg Stein, http://www.lyra.org/

Re: Another type issue.

[Jim Blandy <ji...@zwingli.cygnus.com>]

cmpilato@collab.net writes:
> Karl and I are chatting about the FS function arguments of type 'char
> *', specifically about how those aren't svn_string_t's.  Is there any
> compelling reason why these should *not* be svn_string_t's?  

Legal values for those arguments may not contain null characters.  For
such values, char * is a lot more comfortable to work with than
svn_string_t.  svn_string_t does have some advantages:
- stores length explicitly, to avoid scans
- has separate `length' and `size', to allow strings to be constructed
  piece by piece in linear time
but I don't see that either of these are pressing issues for us at the
moment.  I'd be happy to see profiling that shows otherwise --- after
the first release.

I kind of feel that paths should never be using svn_string_t.  So I
think there are two things we should do:

1) Use char * throughout Subversion for paths.

2) Think through a decent path-handling library, that will actually
   protect code from Windows / Unix / VMS differences, etc., based on
   plain old char *.  Surely such libraries have been designed many
   times.