Fwd: 7050570: (fs) FileSysteProvider fails to initializes if run with file.encoding set to Cp037

[Ulf Zibis]

Am 27.08.2012 12:58, schrieb Alan Bateman:
> On 27/08/2012 10:53, Ulf Zibis wrote:
>> Hi,
>>
>> shouldn't we better code? :
>> -53         if (this.defaultDirectory[0] != '/') {
>> +53         if (this.defaultDirectory[0] != (byte)'/') {
>> It would make it more clear to the reader, that only single bytes are compared here, and maybe 
>> would prevent from the problem when "source code encoding != platform encoding".
>> At least there should be an explaining comment.
>>
>> But why not simply? :
>>   51         this.provider = provider;
>>   52         String normalizedDir = UnixPath.normalizeAndCheck(dir);
>>   53         if (normalizedDir.charAt(0) != '/') {
>>   54             throw new RuntimeException("default directory must be absolute");
>>   55         }
>>   52         this.defaultDirectory = Util.toBytes(normalizedDir);
>>
>> Anyway there already is UnixPath.isAbsolute() to use.
> Thanks for helping to review this.
>
> One thing to say that this is FileSystem creation time so we need to be careful about invoking 
> instance methods on UnixPath until initialization it complete.

You are right, we would need a static isAbsolute(String path), which could be used from both 
classes, like it is already the case for normalizeAndCheck(dir).

> Also given the assumptions then I don't think it matters here whether it's byte 0 or char 0

It matters, if sun.jnu.encoding is a double-byte coding.
... and compared to the anyway processed normalizeAndCheck(dir), performance doesn't matter here.

> but it's the bytes that are used for the syscalls and so if byte 0 is '/' then it's considered an 
> absolute path by the operating system. However a comment to explain what is going on seems 
> necessary so we should do.

:-)

>> Similar in SolarisUserDefinedFileAttributeView.
>> But anyway better code:
>>
>>   45     private static final byte[] HERE = Util.toBytes(".");
>>   46
>>   47     private byte[] nameAsBytes(UnixPath file, String name) throws IOException {
>>   48         byte[] bytes = Util.toBytes(name);
>>   49         //  "", "." and ".." not allowed
>>   50         if (bytes.length == 0 || (bytes[0] == '.' &&
>>   51                 (bytes.length == 1 || (bytes.length == 2 && bytes[1] == '.'))) {
>>   52       ....
> This hasn't been changed but I think you are saying that the nested "if" isn't necessary. This 
> seems a good clean-up to me too - thanks!

Yes, and additionally I wanted to say, that constant HERE should be initialized double-byte encoding 
proof.

I still do not see any reasonable why this UnixFileSystem-API is partly processing chars vs. bytes.
E.G compare UnixPath.normalize(...) vs. UnixPath.initOffsets().

A last thought: I don't like the naming: Util.toString(...). toString has a dedicated meaning in the 
Java world, which is not meant here. What about
- String jnuDecode(byte[])
- byte[] jnuEncode(String)

-Ulf

-------------- next part --------------
An HTML attachment was scrubbed...
URL: http://mail.openjdk.java.net/pipermail/nio-dev/attachments/20120827/42235749/attachment.html