RFR : 8196578 : enhance errno_to_string function in os.cpp with some additional errno texts from AIX 7.1

Thu Feb 8 17:19:26 UTC 2018

After discussing this off-line with Matthias and Goetz, I withdraw my
opposition to this patch.

Still not a big fan, but if everyone else (including David) is okay with
this patch, I am too.

Kind Regards, Thomas

On Fri, Feb 2, 2018 at 10:20 AM, Thomas Stüfe <thomas.stuefe at gmail.com>
wrote:

>
>
> On Fri, Feb 2, 2018 at 9:57 AM, David Holmes <david.holmes at oracle.com>
> wrote:
>
>> While I did not do an exhaustive check of the existing codes even the
>> ones under
>>
>> // The following enums are not defined on all platforms.
>>
>> are at least defined by POSIX (even if just listed as "Reserved").
>>
>> So I am still reluctant to introduce OS specific codes into a shared
>> file. Plus there's the problem of different OS having different meanings
>> for the same error code - suggesting per-OS specialization might be useful
>> (but tricky to implement).
>>
>> That said I have to re-question whether we should be maintaining this
>> explicit string mapping table anyway? strerror() is not thread-safe but
>> strerror_l() seems to be, or at worst we need buffer management with
>> strerror_r(). I know this topic has arisen before ...
>>
>>
> How about we build the string table dynamically at process start by
> iterating the first n errnos and calling strerror() :) Just kidding.
>
> Yes, I admit this table starts to feel weird. Original discussions were
> here: https://bugs.openjdk.java.net/browse/JDK-8148425
>
> I originally just wanted a static translation of errno numbers to
> literalized errno constants (e.g. ETOOMANYREFS => "ETOOMANYREFS"),
> because in 99% of cases where we call os::strerror() we do this to print
> log output for developers, and as a developer I find "ETOOMANYREFS" far
> more succinct than whatever strerror() returns. This would also bypass any
> localization issues. If I see "ETOOMANYREFS" in a log file I immediately
> know this is an error code from the libc, and can look it up in the man
> page or google it. But when I read "Too many references: can't splice" -
> potentially in Portuguese :) - I would have to dig a bit until I find out
> what is actually happening.
>
> Of course, there are cases where we want the human readable, localized
> text, but those cases are rarer and could be rewritten to use strerror_r.
>
> Just my 5 cent.
>
> ..Thomas
>
> Cheers,
>> David
>>
>> On 2/02/2018 6:40 PM, Thomas Stüfe wrote:
>>
>>> On Fri, Feb 2, 2018 at 9:02 AM, Baesken, Matthias <
>>> matthias.baesken at sap.com>
>>> wrote:
>>>
>>>
>>>>     - I do not really like spamming a shared file with AIX specific
>>>> errno
>>>>
>>>>     codes.
>>>>
>>>>
>>>>
>>>> Hi, I wrote  “for a few errnos ***we find*** on AIX 7.1”   ,  not that
>>>> they are  AIX ***specific***.
>>>>
>>>> Checked the first few added ones :
>>>>
>>>>
>>>>
>>>> 1522     // some more errno numbers from AIX 7.1 (some are also
>>>> supported
>>>> on Linux)
>>>>
>>>> 1523     #ifdef ENOTBLK
>>>>
>>>> 1524     DEFINE_ENTRY(ENOTBLK, "Block device required")
>>>>
>>>> 1525     #endif
>>>>
>>>> 1526     #ifdef ECHRNG
>>>>
>>>> 1527     DEFINE_ENTRY(ECHRNG, "Channel number out of range")
>>>>
>>>> 1528     #endif
>>>>
>>>> 1529     #ifdef ELNRNG
>>>>
>>>> 1530     DEFINE_ENTRY(ELNRNG, "Link number out of range")
>>>>
>>>> 1531     #endif
>>>>
>>>>
>>>>
>>>> According to
>>>>
>>>>
>>>>
>>>> http://www.ioplex.com/~miallen/errcmp.html
>>>>
>>>>
>>>>
>>>> ENOTBLK – found on AIX, Solaris, Linux, …
>>>>
>>>> ECHRNG   -  found on  AIX, Solaris, Linux
>>>>
>>>> ELNRNG   -  found on AIX, Solaris, Linux
>>>>
>>>>
>>>>
>>>>
>>> The argument can easily made in the other direction. Checking the last n
>>> errno codes I see:
>>>
>>> AIX, MAC +    #ifdef EPROCLIM
>>> AIX only +    #ifdef ECORRUPT
>>> AIX only  +    #ifdef ESYSERROR
>>> AIX only +    DEFINE_ENTRY(ESOFT, "I/O completed, but needs relocation")
>>> AIX, MAC +    #ifdef ENOATTR
>>> AIX only +    DEFINE_ENTRY(ESAD, "Security authentication denied")
>>> AIX  only  +    #ifdef ENOTRUST
>>> ...
>>>
>>>
>>> I would suggest to keep the multi-platform  errnos in os.cpp  just where
>>>> they are .
>>>>
>>>>
>>>>
>>>>
>>> I am still not convinced and like my original suggestion better. Lets
>>> wait
>>> for others to chime in and see what the consensus is.
>>>
>>> Best Regards, Thomas
>>>
>>>
>>>
>>>
>>>     - Can we move platform specific error codes to platform files? Eg by
>>>>     having a platform specific version pd_errno_to_string(),
>>>>     - which has a first shot at translating errno values, and only if
>>>> that
>>>>     one returns no result reverting back to the shared version?
>>>>     -
>>>>
>>>>
>>>>
>>>> Can go through the list of added  errnos and check if there are really a
>>>> few in that exist only on AIX.
>>>>
>>>> If there are a significant number we might do what you suggest , but for
>>>> only a small number I wouldn’t do it.
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> Small nit:
>>>>>
>>>>
>>>>
>>>>>
>>>> - DEFINE_ENTRY(ESTALE, "Reserved")
>>>>>
>>>>
>>>> + DEFINE_ENTRY(ESTALE, "No filesystem / stale NFS file handle")
>>>>>
>>>>
>>>>
>>>>>
>>>> I like the glibc text better, just "Stale file handle". NFS seems too
>>>>>
>>>> specific, can handles for other remote file systems not get stale?
>>>>
>>>>
>>>>
>>>> That’s fine with me, I can change this to what you suggest.
>>>>
>>>>
>>>>
>>>> Best regards, Matthias
>>>>
>>>>
>>>>
>>>>
>>>>
>>>> *From:* Thomas Stüfe [mailto:thomas.stuefe at gmail.com]
>>>> *Sent:* Donnerstag, 1. Februar 2018 18:38
>>>> *To:* Baesken, Matthias <matthias.baesken at sap.com>
>>>> *Cc:* hotspot-dev at openjdk.java.net; ppc-aix-port-dev at openjdk.java.net
>>>> *Subject:* Re: RFR : 8196578 : enhance errno_to_string function in
>>>> os.cpp
>>>>
>>>> with some additional errno texts from AIX 7.1
>>>>
>>>>
>>>>
>>>> Hi Matthias,
>>>>
>>>>
>>>>
>>>> This would probably better discussed in hotspot-runtime, no?
>>>>
>>>>
>>>>
>>>> The old error codes and their descriptions were Posix (
>>>> http://pubs.opengroup.org/onlinepubs/000095399/basedefs/errno.h.html).
>>>> I
>>>> do not really like spamming a shared file with AIX specific errno codes.
>>>> Can we move platform specific error codes to platform files? Eg by
>>>> having a
>>>> platform specific version pd_errno_to_string(), which has a first shot
>>>> at
>>>> translating errno values, and only if that one returns no result
>>>> reverting
>>>> back to the shared version?
>>>>
>>>>
>>>>
>>>> Small nit:
>>>>
>>>>
>>>>
>>>> - DEFINE_ENTRY(ESTALE, "Reserved")
>>>>
>>>> + DEFINE_ENTRY(ESTALE, "No filesystem / stale NFS file handle")
>>>>
>>>>
>>>>
>>>> I like the glibc text better, just "Stale file handle". NFS seems too
>>>> specific, can handles for other remote file systems not get stale?
>>>>
>>>> Kind Regards, Thomas
>>>>
>>>>
>>>>
>>>> On Thu, Feb 1, 2018 at 5:16 PM, Baesken, Matthias <
>>>> matthias.baesken at sap.com> wrote:
>>>>
>>>> Hello , I enhanced  the  errno - to -  error-text mappings   in os.cpp
>>>>   for a few errnos we find on AIX 7.1 .
>>>> Some of these added errnos are found as well on  Linux (e.g. SLES 11 /
>>>> 12
>>>> ).
>>>>
>>>> Could you please check and review ?
>>>>
>>>> ( btw. there is good cross platform  info about the errnos at
>>>> http://www.ioplex.com/~miallen/errcmp.html     )
>>>>
>>>> Bug :
>>>>
>>>> https://bugs.openjdk.java.net/browse/JDK-8196578
>>>>
>>>> Webrev :
>>>>
>>>> http://cr.openjdk.java.net/~mbaesken/webrevs/8196578/
>>>>
>>>>
>>>>
>>>> Best regards, Matthias
>>>>
>>>>
>>>>
>>>>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/ppc-aix-port-dev/attachments/20180208/3157a369/attachment-0001.html>