hadoop-common-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From malcolm <malcolm.kaval...@oracle.com>
Subject Re: Solaris Port
Date Thu, 11 Dec 2014 16:02:19 GMT
Fine with me, I volunteer to do this, if accepted.

On 12/11/2014 05:48 PM, Allen Wittenauer wrote:
> sys_errlist was removed for a reason.  Creating a fake sys_errlist on Solaris will mean
the libhadoop.so will need to be tied a specific build (kernel/include pairing) and therefore
limits upward mobility/compatibility.  That doesn’t seem like a very good idea.
> IMO, switching to strerror_r is much preferred, since other than the brain-dead GNU libc
version, is highly portable and should work regardless of the kernel or OS in place.
> On Dec 11, 2014, at 5:20 AM, malcolm <malcolm.kavalsky@oracle.com> wrote:
>> FYI, there are a couple more files that reference sys_errlist directly (not just
terror within exception.c) , but also hdfs_http_client.c and NativeiO.c
>> On 12/11/2014 07:38 AM, malcolm wrote:
>>> Hi Colin,
>>> Exactly, as you noticed, the problem is the thread-local buffer needed to return
from terror.
>>> Currently, terror just returns a static string from an array, this is fast, simple
and error-proof.
>>> In order to use strerror_r inside terror,  would require allocating a buffer
inside terror  and depend on the caller to free the buffer after using it, or to pass a buffer
to terrror (which is basically the same as strerror_r, rendering terror redundant).
>>> Both cases require modification outside terror itself, as far as I can tell,
no simple fix. Unless you have an alternative which I haven't thought of ?
>>> As far as I can tell, we have two choices:
>>> 1. Remove terror and replace calls with strerror_r, passing a buffer from the
>>>     Advantage: a more modern portable interface.
>>>     Disadvantage: All calls to terror need to be modified, though all seem to
be in a few files as far as I can tell.
>>> 2. Adding a sys_errlist array (ifdeffed for Solaris)
>>>     Advantage: no change to any calls to terror
>>>     Disadvantage: 2 additional files added to source tree (.c and .h) and some
minor ifdefs only used for Solaris.
>>> I think it is more a question of style than anything else, so I leave you to
make the call.
>>> Thanks for your patience,
>>> Malcolm
>>> On 12/10/2014 09:54 PM, Colin McCabe wrote:
>>>> On Wed, Dec 10, 2014 at 2:31 AM, malcolm <malcolm.kavalsky@oracle.com>
>>>>> Hi Colin,
>>>>> Thanks for the hints around JIRAs.
>>>>> You are correct errno still exists, however sys_errlist does not.
>>>>> Hadoop uses a function terror (defined in exception.c) which indexes
>>>>> sys_errlist by errno to return the error message from the array. This
>>>>> function is called 26 times in various places (in 2.2)
>>>>> Originally, I thought to replace all calls to terror with strerror, but
>>>>> there can be issues with multi-threading (it returns a buffer which can
>>>>> overwritten), so it seemed simpler just to recreate the sys_errlist message
>>>>> array.
>>>>> There is also a multi-threaded version strerror_r where you pass the
>>>>> as a parameter, but this would necessitate changing every call to terror
>>>>> with mutiple lines of code.
>>>> Why don't you just use strerror_r inside terror()?
>>>> I wrote that code originally.  The reason I didn't want to use
>>>> strerror_r there is because GNU libc provides a non-POSIX definition
>>>> of strerror_r, and forcing it to use the POSIX one is a pain. But you
>>>> can do it.  You also will require a thread-local buffer to hold the
>>>> return from strerror_r, since it is not guaranteed to be static
>>>> (although in practice it is 99% of the time-- another annoyance with
>>>> the API).

View raw message