RFR: 8067951: System.loadLibrary cannot find library when path contains quoted entry

[Ivan Gerasimov]

Hi everyone!

May I assume the last webrev is good to go?

- It uses static final in the if clause, so that the additional code is 
eliminated on Unix;
- Quotation marks are used as new delimiter, so it's impossible for them 
to collide with whatever is found in the property value;
- The last loop is as small as was the original, though handles empty 
paths more accurately.

Sincerely yours,
Ivan

On 07.01.2015 0:13, Ivan Gerasimov wrote:
> 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
>>>>
>>>>
>>>
>>
>>
>>
>
>
>