RFR(xxs): 8203680: os::stat() on Posix platform does not need to copy input path

David Holmes david.holmes at oracle.com
Tue Jun 19 08:00:16 UTC 2018


On 19/06/2018 5:44 PM, Thomas Stüfe wrote:
> Hi David,
> 
> thanks for the review!
> 
> new version:
> 
> http://cr.openjdk.java.net/~stuefe/webrevs/8203680-os-stat-posix-should-not-create-copy/webrev.01/webrev/
> 
> I moved the comment from os_posix.cpp to os.hpp, nothing else changed.

s/os_posix.cpp/os_windows.cpp/ :)

Updated comment looks good - thanks.

> Yes, I'd assume stat(2) works the same on all platforms. In any case,
> my patch does not change this - the code calling stat() was equal on
> all posix platforms, I just removed the unnecessary argument copy.

Well, you're also now relying on the native stat to handle any 
excessively long path names ... there may be some subtle differences 
there. And you're now accepting path names that are twice as long as 
before (MAX_PATH was 2K, but PATH_MAX is 4K).

David

> Thanks, Thomas
> 
> 
> 
> On Tue, Jun 19, 2018 at 9:00 AM, David Holmes <david.holmes at oracle.com> wrote:
>> Hi Thomas,
>>
>> Looks fine.
>>
>> I'd probably document the semantics for native_path in os.hpp rather than
>> mentioning Posix in os_windows.cpp though.
>>
>> I assume the system stat function is the same on Linux, BSD, OS X, AIX and
>> Solaris?
>>
>> Thanks,
>> David
>>
>>
>> On 18/06/2018 12:13 AM, Thomas Stüfe wrote:
>>>
>>> Hi,
>>>
>>> may I get reviews for this tiny cleanup/fix.
>>>
>>> Bug: https://bugs.openjdk.java.net/browse/JDK-8203680
>>> patch:
>>> http://cr.openjdk.java.net/~stuefe/webrevs/8203680-os-stat-posix-should-not-create-copy/webrev.00/webrev/
>>>
>>> os::stat() on all Posix platforms copies the input path into a fixed
>>> sized temp buffer, thereby risking truncation, only to call
>>> os::native_path() which is a noop on all Posix platforms.
>>>
>>> This patch replaces this with a straight call to stat(2), so no
>>> truncation anymore. Also, it unifies the coding in os_posix.cpp.
>>>
>>> Thanks, Thomas
>>>
>>


More information about the hotspot-runtime-dev mailing list