<i18n dev> RFR 9: 8166148 Fix for JDK-8165936 broke Solaris builds

Roger Riggs Roger.Riggs at Oracle.com
Thu Sep 15 19:16:21 UTC 2016


Please review a fix for build breakage on Solaris.
The NAME_MAX value does not seem to add much value in this case and is 
not defined on all supported platforms.

Webrev:
    http://cr.openjdk.java.net/~rriggs/webrev-max-8166148/
Issue:
    https://bugs.openjdk.java.net/browse/JDK-8166148

Thanks, Roger


On 9/15/2016 1:39 PM, Roger Riggs wrote:
> Hi Thomas,
>
> It looks like NAME_MAX is not defined on Solaris and breaks the build 
> on Solaris. [1]
>
> An  alternative would be to conditionally use PATH_MAX.
>
> But I think it would be reasonable to just remove the use of NAME_MAX, 
> since the following
> code increases the value to at least 1024, which should be safe.
>
> If you don't have time to address this, I can propose a fix.
>
> Roger
>
> [1] 8166148  fix for JDK-8165936 broke solaris builds 
> <https://bugs.openjdk.java.net/browse/JDK-8166148>
>
>
> On 9/14/2016 2:34 AM, Thomas Stüfe wrote:
>> Hi all,
>>
>> thanks for the reviews. Here is version two:
>>
>> http://cr.openjdk.java.net/~stuefe/webrevs/8165936-Potential-Heap-buffer-overflow-when-seaching-timezone-info-files/webrev.01/webrev/ 
>> <http://cr.openjdk.java.net/%7Estuefe/webrevs/8165936-Potential-Heap-buffer-overflow-when-seaching-timezone-info-files/webrev.01/webrev/> 
>>
>>
>> Only cosmetic changes:
>> - made code pre-c99 compatible
>> - consistently use dirent64
>> - fix indentation in ifs
>> - removed black between malloc and cast
>>
>> Kind Regards, Thomas
>>
>>
>>
>> On Tue, Sep 13, 2016 at 5:25 PM, Masayoshi Okutsu 
>> <masayoshi.okutsu at oracle.com <mailto:masayoshi.okutsu at oracle.com>> 
>> wrote:
>>
>>     Looks good to me. Thank you for fixing this bug!
>>
>>     Masayoshi
>>
>>
>>
>>     On 9/13/2016 11:49 PM, Thomas Stüfe wrote:
>>
>>         Hi Christoph, thanks for your review! Yes, I can remove the 
>> blank.
>>
>>         Kind Regards, Thomas
>>
>>         On Tue, Sep 13, 2016 at 2:35 PM, Langer, Christoph
>>         <christoph.langer at sap.com <mailto:christoph.langer at sap.com>
>>
>>             wrote:
>>             Hi Thomas,
>>
>>             your change looks good. I'm also forwarding this to
>>             i18n-dev as issues in
>>             TimeZone implementation are mostly handled there.
>>
>>             One remark: Can you take the opportunity to also remove
>>             the blank between
>>             the cast and malloc in line 150: "(struct dirent64 *)
>>             malloc..."?
>>
>>             Unfortunately I'm no reviewer, so you still need an
>>             official review.
>>
>>             Best regards
>>             Christoph
>>
>>                 -----Original Message-----
>>                 From: core-libs-dev
>>                 [mailto:core-libs-dev-bounces at openjdk.java.net
>> <mailto:core-libs-dev-bounces at openjdk.java.net>] On
>>
>>             Behalf
>>
>>                 Of Thomas Stüfe
>>                 Sent: Dienstag, 13. September 2016 12:54
>>                 To: Java Core Libs <core-libs-dev at openjdk.java.net
>> <mailto:core-libs-dev at openjdk.java.net>>
>>                 Subject: RFR(xs): 8165936: Potential Heap buffer
>>                 overflow when seaching
>>                 timezone info files
>>
>>                 Dear all,
>>
>>                 please take a look at this small change:
>>
>>                 Bug: https://bugs.openjdk.java.net/browse/JDK-8165936
>> <https://bugs.openjdk.java.net/browse/JDK-8165936>
>>                 Webrev:
>> http://cr.openjdk.java.net/~stuefe/webrevs/8165936-
>> <http://cr.openjdk.java.net/%7Estuefe/webrevs/8165936->
>>
>>             Potential-Heap-buffer-
>>
>> overflow-when-seaching-timezone-info-files/webrev.00/webrev/
>>
>>                 readdir_r is used to iterate over the content of a
>>                 system directory, but
>>                 the buffer passed to it is too small: Its size should
>>                 include the size of
>>                 the dirent structure itself (minus the d_name member).
>>
>>                 The fix also now checks the return code of pathconf(),
>>                 and if pathconf()
>>                 returns an error, falls back to the NAME_MAX compile
>>                 time constant.
>>                 Finally, it imposes a minimum size for the buffer,
>>                 because on older
>>
>>             System
>>
>>                 V systems NAME_MAX may be surprisingly small and
>>                 readdir_r will not check
>>                 the output buffer size. I think it is better to err on
>>                 the safe side
>>
>>             here.
>>
>>                 Kind Regards, Thomas
>>
>>
>>
>



More information about the i18n-dev mailing list