byte -> char conversions in property setting code

[Daniel Rall <dl...@collab.net>]

On Tue, 01 May 2007, Hyrum K. Wright wrote:

> Blair Zajac wrote:
...
> >>>> While I'm not absolutely sure what the Right Thing is here, getting
> >>>> the byte[] representation of a Java String using the JVM's default
> >>>> encoding (which unfortunately might not be UTF-8), then basically
> >>>> transforming each byte in that array into a character, is definitely
> >>>> not correct.
> >>>
> >>> To clarify: That's what we used to do.
> >>>
> >>> Now we seem to do the opposite.  However, if we don't know what
> >>> encoding to create a String from using those bytes, using the JVM's
> >>> default encoding may not be correct, either.
> >>
> >> Can't we use
...
> > value.getBytes( "UTF-8" );
> 
> The attached patch uses a byte array instead of a String to move the
> property values from the Java layer to the C++ layer.  I have a little
> misgiving about using a ClientException, but I don't see a better way to
> implement the error handling when specifying the UTF-8 encoding.

--- subversion/bindings/javahl/native/SVNClient.cpp	(revision 24864)
+++ subversion/bindings/javahl/native/SVNClient.cpp	(working copy)
...
 void SVNClient::propertySet(const char *path, const char *name,
-                            const char *value, bool recurse, bool force)
+                            JNIByteArray &value, bool recurse, bool force)
 {
     Pool requestPool;
     SVN_JNI_NULL_PTR_EX(path, "path", );
     SVN_JNI_NULL_PTR_EX(name, "name", );
-    SVN_JNI_NULL_PTR_EX(value, "value", );
-    svn_string_t *val = svn_string_create(value, requestPool.pool());
+    if (value.isNull())
+    {
+        JNIUtil::throwNullPointerException("value");
+        return;
+    }
+    svn_string_t *val = svn_string_ncreate((const char *)value.getBytes(),
                                            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Is this really kosher?  An element from an array of bytes from a UTF-8
string may or may not correspond to a UTF-8 character.  This is
similar to what we used to be doing -- does it "just work" for some
reason?

+                                           value.getLength(),
+                                           requestPool.pool());
     propertySet(path, name, val, recurse, force, SVN_INVALID_REVNUM);
 }