Hi,
Hi Richard, couple comments after a quick scan.
Thanks for your comments - only just had a post Javaone chance to look at
this stuff. I've pushed an update to:
*http://cr.openjdk.java.net/~rwarburton/string-patch-webrev-7
http://cr.openjdk.java.net/~rwarburton/string-patch-webrev-7*
(1) #474, the IOBE probably is no longer needed in case of ByteBuffer.
parameter.
Fixed in new push.
(2) for methods that have the ByteBuffer as input, it would be desirable to
specify clearly that
the bytes are read from position to limit, and whether or not the
position will be advanced
after decoding/encoding (important).
Added Javadoc in new push.
(3) getBytes(byte[], offset, charset)
while I understand it might be useful to have such a method in
certain circumstance, it is
usually complicated to make it right/easy for user to actually use
it. Consider the fact that
the returned number of bytes encoded has no straightforward
connection to how many
underlying chars have been encoded (so the user of this method really
have no idea how
many underlying chars have been encoded into the dest buffer, is
that enough? how big
the buffer need to be to encode the whole string? ) especially
the possibility that the last
couple byte space might be short of encoding a char. Not like the
getChars(), which has
a easy, clear and direct link between the out going chars and
underlying chars. I would
suggest it might be better to leave it out.
I think you raise some good points here. In my mind this is how it works,
let me know what you think.
The client code of this API call should be able to infer the state of the
byte[] - the offset parameters is in terms of bytes and refers to the
destination buffer. If you want to know how many bytes have been used by
the encoding, the method returns an int that shows the length.
You can query the length of the String, which gives you the number of
outbound chars.
(4) StringCoding.decode() #239 remaining() should be used to return
limit - position?
Fixed in new push.
(5) in case of untrusted, it might be more straightforward to get all
bytes out of the buffer
first (you are allocating a byte buffer here anyway, I don;t see
obvious benefit to get a
direct buffer, btw) and then pass it to the original/existing
byte[]-char[] decoding
implementation. We probably will take a deep look at the
implementation later when
the public api settled.
Yes - I was hoping to remove the additional bytebuffer allocation within
the method. I'm still not sure of the security implications around removing
this code though.
regards,
Richard Warburton
http://insightfullogic.com
@RichardWarburto http://twitter.com/richardwarburto
