Hi Ivan,

Yes, it looks better and should run faster as it doesn't need to worry about 
quote
in second loop. I was a little hesitated when suggested to replace the ps with 
0,
though I'm pretty much sure a \u0000 should never be a legal character in a
windows' path :-)

Anyone can think about any possibility of having an embedded \0 character in 
path?

Nitpick:

the "posCount" probably can be calculated in the "quote handling" loop ?

    if (ClassloaderHelper.allowQuotedPathElements ....) {
         ...
         if (ch ==  ps) { ch = '\0'; psCount++; }
         buf[bufLen++] = ch;
         ...
    } else {
        for (int i = ldPath.indexOf(ps); i >=0 ...) {
        }
    }

otherwise, it looks fine for me.

-Sherman

On 01/06/2015 05:49 AM, Ivan Gerasimov wrote:
Hi Sherman!

I took your suggestion and rewrote the method to moved the logic, which removes 
the quotes to the top.
I think the code became cleaner, so thank you for suggestion!

Here's the updated webrev:
http://cr.openjdk.java.net/~igerasim/8067951/5/webrev/

Sincerely yours,
Ivan


On 06.01.2015 0:12, Xueming Shen wrote:
On 01/05/2015 12:41 PM, Ivan Gerasimov wrote:
Thanks Sherman!

On 05.01.2015 22:10, Xueming Shen wrote:

Just wonder if we really need that "inQuotes" logic here? A straightforward 
approach might
be "every time you have a quote, skip everything until you hit another one, 
when counting,
or copy everything into the buffer until hit another one, when copying" ?

I agree it would work, but, in my opinion, it would be a bit more complicated.
The counting loop would look something like this:
------------------------------------
        outerLoop: for (int i = 0; i < ldLen; ++i) {
            char ch = ldPath.charAt(i);
            if (mayBeQuoted && ch == '\"') {
                thereAreQuotes = true;
                for (++i; i < ldLen; ++i) {
                    if (ldPath.charAt(i) == '\"') {
                        continue outerLoop;
                    }
                }
                break; // unpaired quote
            } else if (ch == ps) {
                psCount++;
            }
        }
------------------------------------
which is 3 lines longer, comparing to the loop with inQuotes flag.


It does not seem like we are doing anything special for "unpaired quote" (just 
ignore it?),
if that is the case, you probably don't need to do anything for it, the code 
could be
something like

        for (int i = 0; i < ldLen; ++i) {
            char ch = ldPath.charAt(i);
            if (mayBeQuoted && ch == '\"') {
                thereAreQuotes = true;
                while (++i < ldLen && ldPath.charAt(i) != '\"') {}
            } else if (ch == ps) {
                psCount++;
            }
        }

I have not tried to debug the code though :-) Just an opinion here.

-Sherman




Reply via email to