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

Thomas Stüfe thomas.stuefe at gmail.com
Tue Jun 19 08:26:06 UTC 2018


Ah, you are right. Still, I assume this would be no issue (not in a
negative way, at least).

..Thomas

On Tue, Jun 19, 2018 at 10:00 AM, David Holmes <david.holmes at oracle.com> wrote:
> 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