Mailing-List: contact common-dev-help@hadoop.apache.org; run by ezmlm
Precedence: bulk
Reply-To: common-dev@hadoop.apache.org
Received-SPF: pass (nike.apache.org: domain of malcolm.kavalsky@oracle.com
 designates 141.146.126.69 as permitted sender)
Message-ID: <54899A17.5020708@oracle.com>
Date: Thu, 11 Dec 2014 15:20:23 +0200
From: malcolm <malcolm.kavalsky@oracle.com>
User-Agent: Mozilla/5.0 (X11; Linux x86_64;
 rv:31.0) Gecko/20100101 Thunderbird/31.3.0
MIME-Version: 1.0
To: common-dev@hadoop.apache.org
Subject: Re: Solaris Port
References: <54858BDF.3030206@oracle.com>
	<CA+qbEUNmZd682nqqa=-KDavb3PL7WAixhYxLUZ3X+DTa6Ratog@mail.gmail.com>
	<54868426.5010206@oracle.com>
	<CA+qbEUNmzqGQZwW=Ed+mVHMMCiZHGVi1C6NgwpMOZYyfFiE_xw@mail.gmail.com>
	<548820ED.2080105@oracle.com>
 <CA+qbEUO5oVTwMRObfKM8Eb+TFLvP+SEsjTDGzjzp0fi65KvK6A@mail.gmail.com>
 <54892DC7.5010909@oracle.com>
In-Reply-To: <54892DC7.5010909@oracle.com>
Content-Type: text/plain; charset=utf-8; format=flowed
Content-Transfer-Encoding: 7bit

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 callee.
>     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> wrote:
>>> 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 be
>>> 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 
>>> buffer
>>> 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).
>>
>>
>