RFR 8167546: enhance os::file_name_strncmp() on Mac OSX

[Gerard Ziemski]

> On Oct 10, 2018, at 12:11 PM, Jiangli Zhou <jiangli.zhou at oracle.com> wrote:
> 
> Hi Gerard,
> 
> 
> On 10/10/18 7:21 AM, Gerard Ziemski wrote:
>> hi Jiangli,
>> 
>> 
>>> On Oct 9, 2018, at 5:10 PM, Jiangli Zhou <jiangli.zhou at oracle.com> wrote:
>>> 
>>> Hi Gerard,
>>> 
>>> The path string may contain more than one path component, which needs to be resolved individually, or os::Posix::realpath() would fail.
>> The “file_name_strncmp(char* s1, char* s2, size_t num)” API takes the “num” argument, which partitions the path string to a singular path, so it will not fail.
> 
> The 'num' argument is the string length of the path string 's1'. 's1' may contain more than one path component and os::Posix::realpath(s1, resolved1, PATH_MAX) would fail in this case. Same for the second os::Posix::realpath(path2, resolved2, PATH_MAX) call. The copied 'path2' may contain single path, truncated path, or multiple paths with truncated path depending on the 'num' of characters copied.
> 
>> 
>> 
>>> Your approach using the fully resolved path is in the right direction. It will address all various path validation issues. The path validation issues occur on all platforms and need to be resolved for all. We can fix the shared SharedPathsMiscInfo::check() code, and avoid duplicating code in platform specific files. In SharedPathsMiscInfo::check() we can do a quick string comparison first as you have in bsd os::file_name_strncmp(). If the quick test fails, we can then use the canonical paths for comparison.
>>> 
>>> To make the code cleaner, we could completely get rid of the separate path comparison that uses SharedPathsMiscInfo::check()/os::file_name_strncmp() and do the path sequence check as part of FileMapInfo::validate_shared_path_table(). That can be done at a later point.
>> It’s your code, so I defer to your decision, but just to make sure I understand what you are saying: the CDS path processing code needs
>> to handle this issue (8167546) as well as 8211723 (which I filed recently), so we need to make these 2 issues work for all platforms and there is some refactoring you have in mind for 8211723, which will end up fixing this issue in the process?
> Yes, 8211723 is a larger scope of the path validation issues and occurring on all platforms. Fixing 8211723 would address the Mac OSX case sensitive issue as well, I think.
>> 
>> If that’s what you mean, then I’m perfectly fine with such approach. I provided this code, which has been extensively reviewed and tested, and which hopefully will be of help to you later.
>> 
>> Can you take over 8167546 (this issue) and mark it a duplicate of 8211723 (and explain to Calvin what happened)?
> Since you have already put a lot of effort in this area, I thought you might be interested in taking this further. I can take over if that's okay with you.
> 
> Thanks a lot for looking into this!

Oh, I thought you wanted it. Personally, I have no problem finishing this as part of 8211723. That enhancement is likely not for JDK12 though, right?


cheers