And here's the update:
http://cr.openjdk.java.net/~igerasim/8067951/6/webrev/
As you suggested, the psCount is calculated in the first loop along the way.
Another change is that a quotation mark is used as the new path separator.
I also thought it's better to add a short comment, as now it may look
confusing at a first glance.
Sincerely yours,
Ivan
On 06.01.2015 21:33, Xueming Shen wrote:
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
