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

Re: svn commit: r24791 - in trunk/subversion/bindings/javahl: native src/org/tigris/subversion/javahl src/org/tigris/subversion/javahl/tests

From: Hyrum K. Wright <hyrum_wright_at_mail.utexas.edu>
Date: 2007-05-01 19:22:32 CEST

Blair Zajac wrote:
>
> On Apr 26, 2007, at 7:07 PM, Blair Zajac wrote:
>
>>
>> On Apr 26, 2007, at 6:04 PM, Daniel Rall wrote:
>>
>>> On Thu, 26 Apr 2007, Daniel Rall wrote:
>>>
>>>> On Thu, 26 Apr 2007, Hyrum K. Wright wrote:
>>>>
>>>>> Daniel Rall wrote:
>>>>>> On Thu, 26 Apr 2007, hwright@tigris.org wrote:
>>>>>> ...
>>>>>>> JavaHL: Eliminate duplicate native implementations of
>>>>>>> propertyCreate(), and
>>>>>>> implement one version as a wrapper around the other. Also,
>>>>>>> modify the
>>>>>>> properties test to use propertyCreate() as well as propertySet().
>>>>>> ...
>>>>>>> ---
>>>>>>> trunk/subversion/bindings/javahl/native/org_tigris_subversion_javahl_SVNClient.cpp
>>>>>>> (original)
>>>>>>> +++
>>>>>>> trunk/subversion/bindings/javahl/native/org_tigris_subversion_javahl_SVNClient.cpp
>>>>>>> Thu Apr 26 09:00:39 2007
>>>> ...
>>>>>>> - JNIByteArray value(jvalue);
>>>>>>> - if (JNIUtil::isExceptionThrown())
>>>>>>> - return;
>>>>>>> -
>>>>>>> - cl->propertyCreate(path, name, (const char *)value.getBytes(),
>>>>>>> - jrecurse ? true:false,
>>>>>>> - jforce ? true:false);
>>>>>>> -}
>>>> ...
>>>>>>> ---
>>>>>>> trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/SVNClient.java
>>>>>>> (original)
>>>>>>> +++
>>>>>>> trunk/subversion/bindings/javahl/src/org/tigris/subversion/javahl/SVNClient.java
>>>>>>> Thu Apr 26 09:00:39 2007
>>>>>>> @@ -1447,9 +1447,12 @@
>>>>>>> * @throws ClientException
>>>>>>> * @since 1.2
>>>>>>> */
>>>>>>> - public native void propertyCreate(String path, String name,
>>>>>>> byte[] value,
>>>>>>> - boolean recurse, boolean
>>>>>>> force)
>>>>>>> - throws ClientException;
>>>>>>> + public void propertyCreate(String path, String name, byte[]
>>>>>>> value,
>>>>>>> + boolean recurse, boolean force)
>>>>>>> + throws ClientException
>>>>>>> + {
>>>>>>> + propertyCreate(path, name, new String(value), recurse,
>>>>>>> force);
>>>>>>> + }
>>>>>> ...
>>>>>>
>>>>>> Isn't "new String(value)" doing an implicit byte -> char
>>>>>> conversion in
>>>>>> the JVM's default character set? Could this be lossy?
>>>>>>
>>>>>> It might be safer to make this change the other way around...
>>>>>
>>>>> Sure. The motivation for doing it this way was consistency with a
>>>>> similar construct in propertySet(). I'd be happy to change them
>>>>> both if
>>>>> needed.
>>>>
>>>> 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
>>
>> http://java.sun.com/j2se/1.5.0/docs/api/java/lang/String.html#getBytes(java.lang.String)
>>
>
> And 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.

-Hyrum

[[[
JavaHL: Use a byte array instead of a String to pass property values
from the Java layer to the C++ layer.

Sugggested by: dlr
               blair

[ in subversion/bindings/javahl ]
* native/org_tigris_subversion_javahl_SVNClient.cpp
  (Java_org_tigris_subversion_javahl_SVNClient_propertySet):
  Update arguments to use a byte array.

* native/SVNClient.h,
  native/SVNClient.cpp
  (propertySet): Accept a JNIByteArray in place of a const char * for
  the property value.

* src/org/tigris/subversion/javahl/SVNClient.java
  (propertySet): Use a native implementation for the byte array version.
  (propertySet): Implement the String version as a wrapper around the
  byte array version. Convert the String to bytes by using the UTF-8
  encoding. Catch a potential encoding exception, and re-throw it as a
  ClientException.
]]]

Received on Tue May 1 19:25:57 2007

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