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

[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. Also given the assumptions then I don't think it matters 
here whether it's byte 0 or char 0 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!

-Alan



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