Patchwork [2/3] libstdc++-v3: ::tmpnam depends on uClibc SUSV4_LEGACY

login
register
mail settings
Submitter aldot
Date April 4, 2013, 7:53 p.m.
Message ID <1365105210-16552-3-git-send-email-rep.dot.nop@gmail.com>
Download mbox | patch
Permalink /patch/233932/
State New
Headers show

Comments

aldot - April 4, 2013, 7:53 p.m.
POSIX.1-2008 (SUSv4) marks tmpnam() as obsolescent. As such it is not
available in uClibc unless SUSv4 legacy stuff is enabled.

libstdc++-v3/ChangeLog

2013-03-24  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>

	* include/c_global/cstdio: On uClibc guard ::tmpnam with SUSv4
	legacy availability.

Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
---
 libstdc++-v3/include/c_global/cstdio |    2 ++
 1 file changed, 2 insertions(+)
Gabriel Dos Reis - April 4, 2013, 8 p.m.
On Thu, Apr 4, 2013 at 2:53 PM, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
> POSIX.1-2008 (SUSv4) marks tmpnam() as obsolescent. As such it is not
> available in uClibc unless SUSv4 legacy stuff is enabled.
>
> libstdc++-v3/ChangeLog
>
> 2013-03-24  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>
>
>         * include/c_global/cstdio: On uClibc guard ::tmpnam with SUSv4
>         legacy availability.
>
> Signed-off-by: Bernhard Reutner-Fischer <rep.dot.nop@gmail.com>
> ---
>  libstdc++-v3/include/c_global/cstdio |    2 ++
>  1 file changed, 2 insertions(+)
>
> diff --git a/libstdc++-v3/include/c_global/cstdio b/libstdc++-v3/include/c_global/cstdio
> index fcbec0c..037a668 100644
> --- a/libstdc++-v3/include/c_global/cstdio
> +++ b/libstdc++-v3/include/c_global/cstdio
> @@ -131,7 +131,9 @@ namespace std
>    using ::sprintf;
>    using ::sscanf;
>    using ::tmpfile;
> +#if !defined __UCLIBC__ || defined __UCLIBC_SUSV4_LEGACY__
>    using ::tmpnam;
> +#endif
>    using ::ungetc;
>    using ::vfprintf;
>    using ::vprintf;
> --
> 1.7.10.4
>

Sounds good to me.

-- Gaby
Rainer Orth - April 5, 2013, 9:01 a.m.
Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

>> diff --git a/libstdc++-v3/include/c_global/cstdio b/libstdc++-v3/include/c_global/cstdio
>> index fcbec0c..037a668 100644
>> --- a/libstdc++-v3/include/c_global/cstdio
>> +++ b/libstdc++-v3/include/c_global/cstdio
>> @@ -131,7 +131,9 @@ namespace std
>>    using ::sprintf;
>>    using ::sscanf;
>>    using ::tmpfile;
>> +#if !defined __UCLIBC__ || defined __UCLIBC_SUSV4_LEGACY__
>>    using ::tmpnam;
>> +#endif
>>    using ::ungetc;
>>    using ::vfprintf;
>>    using ::vprintf;
>> --
>> 1.7.10.4
>>
>
> Sounds good to me.

Do we really want to use target-specific macros directly instead of
defining something more abstract either via a configure test or a define
in config/os/uclibc?

	Rainer
Gabriel Dos Reis - April 5, 2013, 9:08 a.m.
On Fri, Apr 5, 2013 at 4:01 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>
>>> diff --git a/libstdc++-v3/include/c_global/cstdio b/libstdc++-v3/include/c_global/cstdio
>>> index fcbec0c..037a668 100644
>>> --- a/libstdc++-v3/include/c_global/cstdio
>>> +++ b/libstdc++-v3/include/c_global/cstdio
>>> @@ -131,7 +131,9 @@ namespace std
>>>    using ::sprintf;
>>>    using ::sscanf;
>>>    using ::tmpfile;
>>> +#if !defined __UCLIBC__ || defined __UCLIBC_SUSV4_LEGACY__
>>>    using ::tmpnam;
>>> +#endif
>>>    using ::ungetc;
>>>    using ::vfprintf;
>>>    using ::vprintf;
>>> --
>>> 1.7.10.4
>>>
>>
>> Sounds good to me.
>
> Do we really want to use target-specific macros directly instead of
> defining something more abstract either via a configure test or a define
> in config/os/uclibc?
>
>         Rainer

What would your suggestion for defineingsomething more abstract that reliably
says whether the feature is deprecated or absent?


>
> --
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University
Rainer Orth - April 5, 2013, 9:13 a.m.
Gabriel Dos Reis <gdr@integrable-solutions.net> writes:

> On Fri, Apr 5, 2013 at 4:01 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>>
>>>> diff --git a/libstdc++-v3/include/c_global/cstdio
>>>> b/libstdc++-v3/include/c_global/cstdio
>>>> index fcbec0c..037a668 100644
>>>> --- a/libstdc++-v3/include/c_global/cstdio
>>>> +++ b/libstdc++-v3/include/c_global/cstdio
>>>> @@ -131,7 +131,9 @@ namespace std
>>>>    using ::sprintf;
>>>>    using ::sscanf;
>>>>    using ::tmpfile;
>>>> +#if !defined __UCLIBC__ || defined __UCLIBC_SUSV4_LEGACY__
>>>>    using ::tmpnam;
>>>> +#endif
>>>>    using ::ungetc;
>>>>    using ::vfprintf;
>>>>    using ::vprintf;
>>>> --
>>>> 1.7.10.4
b>>>>
>>>
>>> Sounds good to me.
>>
>> Do we really want to use target-specific macros directly instead of
>> defining something more abstract either via a configure test or a define
>> in config/os/uclibc?
>>
>>         Rainer
>
> What would your suggestion for defineingsomething more abstract that reliably
> says whether the feature is deprecated or absent?

It seems _GLIBCXX_USE_TMPNAM would be in line with the other macros I
see.  Than either configure could test if tmpnam() is available without
special additional macros or config/os/uclibc/os_config.h could define
it to 0, with a default of 1 (best decided by the libstdc++
maintainers).

The configure route seems cleaner to me, especially given that
Bernhard's rationale for uClibc no longer providing it by default
suggests that other systems might follow in the foreseeable future.

	Rainer
Gabriel Dos Reis - April 5, 2013, 9:23 a.m.
On Fri, Apr 5, 2013 at 4:13 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>
>> On Fri, Apr 5, 2013 at 4:01 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>>> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>>>
>>>>> diff --git a/libstdc++-v3/include/c_global/cstdio
>>>>> b/libstdc++-v3/include/c_global/cstdio
>>>>> index fcbec0c..037a668 100644
>>>>> --- a/libstdc++-v3/include/c_global/cstdio
>>>>> +++ b/libstdc++-v3/include/c_global/cstdio
>>>>> @@ -131,7 +131,9 @@ namespace std
>>>>>    using ::sprintf;
>>>>>    using ::sscanf;
>>>>>    using ::tmpfile;
>>>>> +#if !defined __UCLIBC__ || defined __UCLIBC_SUSV4_LEGACY__
>>>>>    using ::tmpnam;
>>>>> +#endif
>>>>>    using ::ungetc;
>>>>>    using ::vfprintf;
>>>>>    using ::vprintf;
>>>>> --
>>>>> 1.7.10.4
> b>>>>
>>>>
>>>> Sounds good to me.
>>>
>>> Do we really want to use target-specific macros directly instead of
>>> defining something more abstract either via a configure test or a define
>>> in config/os/uclibc?
>>>
>>>         Rainer
>>
>> What would your suggestion for defineingsomething more abstract that reliably
>> says whether the feature is deprecated or absent?
>
> It seems _GLIBCXX_USE_TMPNAM would be in line with the other macros I
> see.  Than either configure could test if tmpnam() is available without
> special additional macros or config/os/uclibc/os_config.h could define
> it to 0, with a default of 1 (best decided by the libstdc++
> maintainers).
>
> The configure route seems cleaner to me, especially given that
> Bernhard's rationale for uClibc no longer providing it by default
> suggests that other systems might follow in the foreseeable future.
>
>         Rainer
>
> --
> -----------------------------------------------------------------------------
> Rainer Orth, Center for Biotechnology, Bielefeld University

sounds reasonable; Bernhard, would you mind amending your patch in
that direction?
aldot - April 5, 2013, 9:48 a.m.
On 5 April 2013 11:23, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
> On Fri, Apr 5, 2013 at 4:13 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>>
>>> On Fri, Apr 5, 2013 at 4:01 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>>>> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>>>>
>>>>>> diff --git a/libstdc++-v3/include/c_global/cstdio
>>>>>> b/libstdc++-v3/include/c_global/cstdio
>>>>>> index fcbec0c..037a668 100644
>>>>>> --- a/libstdc++-v3/include/c_global/cstdio
>>>>>> +++ b/libstdc++-v3/include/c_global/cstdio
>>>>>> @@ -131,7 +131,9 @@ namespace std
>>>>>>    using ::sprintf;
>>>>>>    using ::sscanf;
>>>>>>    using ::tmpfile;
>>>>>> +#if !defined __UCLIBC__ || defined __UCLIBC_SUSV4_LEGACY__
>>>>>>    using ::tmpnam;
>>>>>> +#endif
>>>>>>    using ::ungetc;
>>>>>>    using ::vfprintf;
>>>>>>    using ::vprintf;
>>>>>> --
>>>>>> 1.7.10.4
>> b>>>>
>>>>>
>>>>> Sounds good to me.
>>>>
>>>> Do we really want to use target-specific macros directly instead of
>>>> defining something more abstract either via a configure test or a define
>>>> in config/os/uclibc?
>>>>
>>>>         Rainer
>>>
>>> What would your suggestion for defineingsomething more abstract that reliably
>>> says whether the feature is deprecated or absent?
>>
>> It seems _GLIBCXX_USE_TMPNAM would be in line with the other macros I
>> see.  Than either configure could test if tmpnam() is available without
>> special additional macros or config/os/uclibc/os_config.h could define
>> it to 0, with a default of 1 (best decided by the libstdc++
>> maintainers).
>>
>> The configure route seems cleaner to me, especially given that
>> Bernhard's rationale for uClibc no longer providing it by default
>> suggests that other systems might follow in the foreseeable future.

> sounds reasonable; Bernhard, would you mind amending your patch in
> that direction?

I'll have a look.
Thanks,
aldot - April 11, 2013, 12:04 p.m.
On 5 April 2013 11:48, Bernhard Reutner-Fischer <rep.dot.nop@gmail.com> wrote:
> On 5 April 2013 11:23, Gabriel Dos Reis <gdr@integrable-solutions.net> wrote:
>> On Fri, Apr 5, 2013 at 4:13 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>>> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>>>
>>>> On Fri, Apr 5, 2013 at 4:01 AM, Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>>>>> Gabriel Dos Reis <gdr@integrable-solutions.net> writes:
>>>>>
>>>>>>> diff --git a/libstdc++-v3/include/c_global/cstdio
>>>>>>> b/libstdc++-v3/include/c_global/cstdio
>>>>>>> index fcbec0c..037a668 100644
>>>>>>> --- a/libstdc++-v3/include/c_global/cstdio
>>>>>>> +++ b/libstdc++-v3/include/c_global/cstdio
>>>>>>> @@ -131,7 +131,9 @@ namespace std
>>>>>>>    using ::sprintf;
>>>>>>>    using ::sscanf;
>>>>>>>    using ::tmpfile;
>>>>>>> +#if !defined __UCLIBC__ || defined __UCLIBC_SUSV4_LEGACY__
>>>>>>>    using ::tmpnam;
>>>>>>> +#endif
>>>>>>>    using ::ungetc;
>>>>>>>    using ::vfprintf;
>>>>>>>    using ::vprintf;
>>>>>>> --
>>>>>>> 1.7.10.4
>>> b>>>>
>>>>>>
>>>>>> Sounds good to me.
>>>>>
>>>>> Do we really want to use target-specific macros directly instead of
>>>>> defining something more abstract either via a configure test or a define
>>>>> in config/os/uclibc?
>>>>>
>>>>>         Rainer
>>>>
>>>> What would your suggestion for defineingsomething more abstract that reliably
>>>> says whether the feature is deprecated or absent?
>>>
>>> It seems _GLIBCXX_USE_TMPNAM would be in line with the other macros I
>>> see.  Than either configure could test if tmpnam() is available without
>>> special additional macros or config/os/uclibc/os_config.h could define
>>> it to 0, with a default of 1 (best decided by the libstdc++
>>> maintainers).
>>>
>>> The configure route seems cleaner to me, especially given that
>>> Bernhard's rationale for uClibc no longer providing it by default
>>> suggests that other systems might follow in the foreseeable future.
>
>> sounds reasonable; Bernhard, would you mind amending your patch in
>> that direction?
>
> I'll have a look.

Done with the configure check, looks prettier indeed, will followup once tested.

I would have expected that somebody would tell me that omitting
::tmpnam violates 27.9.2 <cstdio> from the spec but noone yelled at me
yet?
> Thanks,
Paolo Carlini - April 11, 2013, 12:18 p.m.
Hi,

On 04/11/2013 02:04 PM, Bernhard Reutner-Fischer wrote:
> I would have expected that somebody would tell me that omitting 
> ::tmpnam violates 27.9.2 <cstdio> from the spec but noone yelled at me 
> yet? 
Frankly, I didn't because the targets I really care about aren't 
affected. The actual maintainers of this target should speak.

Paolo.
aldot - Nov. 8, 2013, 10:29 a.m.
On 11 April 2013 14:18, Paolo Carlini <paolo.carlini@oracle.com> wrote:
> Hi,
>
>
> On 04/11/2013 02:04 PM, Bernhard Reutner-Fischer wrote:
>>
>> I would have expected that somebody would tell me that omitting ::tmpnam
>> violates 27.9.2 <cstdio> from the spec but noone yelled at me yet?
>
> Frankly, I didn't because the targets I really care about aren't affected.
> The actual maintainers of this target should speak.

Attaching an updated patch that i was using since March (without
regressions) which takes Rainer's comments about _GLIBCXX_USE_TMPNAM
into account.
Ok for trunk?

libstdc++-v3/ChangeLog

2013-03-24  Bernhard Reutner-Fischer  <aldot@gcc.gnu.org>

        * acinclude.m4 (GLIBCXX_CHECK_TMPNAM): New check for tmpnam
        function.
        * configure.ac: Use GLIBCXX_CHECK_TMPNAM.
        * (configure, config.h.in): Regenerate.
        * include/c_global/cstdio: Guard ::tmpnam with _GLIBCXX_USE_TMPNAM
Jonathan Wakely - Nov. 11, 2013, 11:30 a.m.
On 8 November 2013 10:29, Bernhard Reutner-Fischer wrote:
>> On 04/11/2013 02:04 PM, Bernhard Reutner-Fischer wrote:
>>>
>>> I would have expected that somebody would tell me that omitting ::tmpnam
>>> violates 27.9.2 <cstdio> from the spec but noone yelled at me yet?

std::tmpnam, like std::gets, should be killed with fire. If a target C
library doesn't provide it then I have no problem is libstdc++ doesn't
provide it either.

> Attaching an updated patch that i was using since March (without
> regressions) which takes Rainer's comments about _GLIBCXX_USE_TMPNAM
> into account.
> Ok for trunk?

Thanks for following this up.

I'm curious why you use tmpnam("NULL") rather than tmpnam(NULL) or
tmpnam("something")?  Using the string literal "NULL" is a bit
confusing (although not a problem.)

How does __UCLIBC_SUSV4_LEGACY__ get defined?  We'd have a problem if
users defined that at configure time but not later when using the
library.
aldot - Nov. 13, 2013, 9:22 a.m.
On 11 November 2013 12:30, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> On 8 November 2013 10:29, Bernhard Reutner-Fischer wrote:
>>> On 04/11/2013 02:04 PM, Bernhard Reutner-Fischer wrote:
>>>>
>>>> I would have expected that somebody would tell me that omitting ::tmpnam
>>>> violates 27.9.2 <cstdio> from the spec but noone yelled at me yet?
>
> std::tmpnam, like std::gets, should be killed with fire. If a target C
> library doesn't provide it then I have no problem is libstdc++ doesn't
> provide it either.
>
>> Attaching an updated patch that i was using since March (without
>> regressions) which takes Rainer's comments about _GLIBCXX_USE_TMPNAM
>> into account.
>> Ok for trunk?
>
> Thanks for following this up.
>
> I'm curious why you use tmpnam("NULL") rather than tmpnam(NULL) or
> tmpnam("something")?  Using the string literal "NULL" is a bit
> confusing (although not a problem.)

Yea, perhaps that's confusing, let's use "XYZ" instead.
>
> How does __UCLIBC_SUSV4_LEGACY__ get defined?  We'd have a problem if
> users defined that at configure time but not later when using the
> library.
That would be defined by uClibc's configury, but the latest
"commit-6f2faa2" i attached does not mention this anymore, but does
the check in a libc-agnostic manner?
Jonathan Wakely - Nov. 13, 2013, 5:56 p.m.
On 13 November 2013 09:22, Bernhard Reutner-Fischer wrote:
> On 11 November 2013 12:30, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>> How does __UCLIBC_SUSV4_LEGACY__ get defined?  We'd have a problem if
>> users defined that at configure time but not later when using the
>> library.
> That would be defined by uClibc's configury, but the latest
> "commit-6f2faa2" i attached does not mention this anymore, but does
> the check in a libc-agnostic manner?

Yes, but I was concerned about whether the value of that macro can
change between configuring libstdc++ and users compiling code using
libstdc++.  If it could change (e.g. by users compiling with
-D_POSIX_C_SOURCE=200112L or some other feature test macro) then the
value of _GLIBCXX_USE_TMPNAM (which doesn't change) would be
unreliable and we could end up with a "using ::tmpnam" in the library
that causes errors when users compile.

If it's set when configuring uClibc then it is a constant for a given
libstdc++ installation, so the value of _GLIBCXX_USE_TMPNAM is
reliable.  In that case your change is OK to commit (with or without
the "XYZ" change) - thanks.
aldot - Dec. 20, 2013, 12:16 p.m.
On 13 November 2013 18:56, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
> On 13 November 2013 09:22, Bernhard Reutner-Fischer wrote:
>> On 11 November 2013 12:30, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>>> How does __UCLIBC_SUSV4_LEGACY__ get defined?  We'd have a problem if
>>> users defined that at configure time but not later when using the
>>> library.
>> That would be defined by uClibc's configury, but the latest
>> "commit-6f2faa2" i attached does not mention this anymore, but does
>> the check in a libc-agnostic manner?
>
> Yes, but I was concerned about whether the value of that macro can
> change between configuring libstdc++ and users compiling code using
> libstdc++.  If it could change (e.g. by users compiling with
> -D_POSIX_C_SOURCE=200112L or some other feature test macro) then the
> value of _GLIBCXX_USE_TMPNAM (which doesn't change) would be
> unreliable and we could end up with a "using ::tmpnam" in the library
> that causes errors when users compile.
>
> If it's set when configuring uClibc then it is a constant for a given
> libstdc++ installation, so the value of _GLIBCXX_USE_TMPNAM is
> reliable.  In that case your change is OK to commit (with or without
> the "XYZ" change) - thanks.

It is a constant, yes. I will push this after another round of regtests
against current trunk as time permits.

thanks,
aldot - April 8, 2014, 9:10 p.m.
On 20 December 2013 13:16, Bernhard Reutner-Fischer
<rep.dot.nop@gmail.com> wrote:
> On 13 November 2013 18:56, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>> On 13 November 2013 09:22, Bernhard Reutner-Fischer wrote:
>>> On 11 November 2013 12:30, Jonathan Wakely <jwakely.gcc@gmail.com> wrote:
>>>> How does __UCLIBC_SUSV4_LEGACY__ get defined?  We'd have a problem if
>>>> users defined that at configure time but not later when using the
>>>> library.
>>> That would be defined by uClibc's configury, but the latest
>>> "commit-6f2faa2" i attached does not mention this anymore, but does
>>> the check in a libc-agnostic manner?
>>
>> Yes, but I was concerned about whether the value of that macro can
>> change between configuring libstdc++ and users compiling code using
>> libstdc++.  If it could change (e.g. by users compiling with
>> -D_POSIX_C_SOURCE=200112L or some other feature test macro) then the
>> value of _GLIBCXX_USE_TMPNAM (which doesn't change) would be
>> unreliable and we could end up with a "using ::tmpnam" in the library
>> that causes errors when users compile.
>>
>> If it's set when configuring uClibc then it is a constant for a given
>> libstdc++ installation, so the value of _GLIBCXX_USE_TMPNAM is
>> reliable.  In that case your change is OK to commit (with or without
>> the "XYZ" change) - thanks.
>
> It is a constant, yes. I will push this after another round of regtests
> against current trunk as time permits.

Just rebased and saw that sje committed this as svn r207009 for me
since i apparently forgot..
Thanks!
aldot - April 14, 2014, 10:09 a.m.
Jonathan,

I see that you filed LWG 2249 (gets() removal), may i ask you to take
care of the tmpnam removal, too?
The reasoning would be exactly the same as for gets().

TIA && cheers,

http://wg21.cmeerw.net/lwg/issue2249
Jonathan Wakely - April 14, 2014, 10:23 a.m.
On 14 April 2014 11:09, Bernhard Reutner-Fischer wrote:
> Jonathan,
>
> I see that you filed LWG 2249 (gets() removal), may i ask you to take
> care of the tmpnam removal, too?
> The reasoning would be exactly the same as for gets().

The reason to remove gets() was that it's no longer in the C standard,
but as far as I can tell tmpnam() is still in both C and POSIX, so the
reason cannot be the same.

POSIX marks it as obsolescent, which means it is still part of the
standard, but may be removed in a future version.

Patch

diff --git a/libstdc++-v3/include/c_global/cstdio b/libstdc++-v3/include/c_global/cstdio
index fcbec0c..037a668 100644
--- a/libstdc++-v3/include/c_global/cstdio
+++ b/libstdc++-v3/include/c_global/cstdio
@@ -131,7 +131,9 @@  namespace std
   using ::sprintf;
   using ::sscanf;
   using ::tmpfile;
+#if !defined __UCLIBC__ || defined __UCLIBC_SUSV4_LEGACY__
   using ::tmpnam;
+#endif
   using ::ungetc;
   using ::vfprintf;
   using ::vprintf;