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

[Xueming Shen]

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" ?

Since you are making copy (into the temporary buffer) anyway for "quote" case, another
approach might be to pre-scan the input propName first for quote, if there is one, make
a clean copy of the propName without those quotes, then go to the original implementation.
This logic can be implemented at the very beginning as

private static String[] intializePath(String propname) {
    if (ClassLoaderHelper.allowsQuotedPathElements) {
        ...
    }
     ...   // existing implementation
}

-Sherman

On 01/05/2015 10:10 AM, Roger wrote:
> Hi Ivan,
>
> Looks pretty good.
>
> Just a thought on ClassLoader:1754;  If you avoid the local for ClassLoaderHelper.allowsQuotedPathElements
> then the compiler can optimize the code and may do dead code elimination since it is a final static boolean.
> On Mac and Unix it reduces to just the original code.
>
> (ClassLoaderHelper seems like a weak case for a separate class, IMHO)
>
> $.02, Roger
>
>
> On 1/5/2015 12:49 PM, Ivan Gerasimov wrote:
>> Hi Roger!
>>
>> I've updated the webrev similarly to what you've suggested:
>> 1) Presence of quotes is checked when counting the separators.
>> 2) If quotes weren't found, we'll execute the same optimized loop as for Unix.
>>
>> http://cr.openjdk.java.net/~igerasim/8067951/4/webrev/
>>
>> Sincerely yours,
>> Ivan
>>
>>
>> On 05.01.2015 18:40, Roger wrote:
>>> Hi Ivan,
>>>
>>> For this small difference in the implementations, I'd recommend against having
>>> two different source files.  The path initialization function is a one time function and
>>> the performance improvement is not significant.
>>>
>>>  I'd suggest a few comments on your 2nd version[1].
>>>
>>> - The windows check should check the system property or other definitive os check
>>>   and could be better expressed (as Alan suggested) as quotesAllowed.
>>> - in the loop testing for the quote (") can come before the quotesAllowed check to
>>>   speed things up (no need to check if they are allowed if they do not occur).
>>>   This code is unlikely to be executed enough times to optimized.
>>>
>>> Roger
>>>
>>> [1] http://cr.openjdk.java.net/~igerasim/8067951/2/webrev/
>>>
>>> On 1/4/2015 3:23 PM, Ivan Gerasimov wrote:
>>>>
>>>> On 04.01.2015 22:50, Alan Bateman wrote:
>>>>> On 03/01/2015 17:39, Ivan Gerasimov wrote:
>>>>>>
>>>>>> Currently, there are tree variants of ClassLoaderHelper: for Windows, for Unix and for MacOS.
>>>>>> We have to either duplicate code in Unix and MacOS realizations, or introduce another Helper class for initializing paths only, which would have only two realizations: for Windows and all Unixes.
>>>>> When I made the comment then I was thinking of a method such as allowsQuotedPathElements (or a better name) that returns a boolean to indicate if quoting of path elements is allowed or not. That would abstract the capability a bit without needing to do isWindows checks.
>>>>>
>>>> Ah, I see.
>>>> Though, not needing to check for quotes allows a bit more efficient implementation, so splitting the code for different platforms may also make sense.
>>>>
>>>> I did it with another helper class in this webrev:
>>>> http://cr.openjdk.java.net/~igerasim/8067951/3/webrev/
>>>>
>>>> Sincerely yours,
>>>> Ivan
>>>>
>>>>
>>>>> -Alan.
>>>>>
>>>>>
>>>>
>>>
>>>
>>>
>>
>