public inbox for gcc-patches@gcc.gnu.org
 help / color / mirror / Atom feed
From: Martin Sebor <msebor@gmail.com>
To: Jonathan Wakely <jwakely.gcc@gmail.com>
Cc: Jonathan Wakely <jwakely@redhat.com>,
	 libstdc++ <libstdc++@gcc.gnu.org>,
	gcc-patches <gcc-patches@gcc.gnu.org>
Subject: Re: [patch] libstdc++/67173 Fix filesystem::canonical for Solaris 10.
Date: Sat, 12 Sep 2015 19:49:00 -0000	[thread overview]
Message-ID: <55F469CF.9010503@gmail.com> (raw)
In-Reply-To: <CAH6eHdTjPFof-+ENBLytMsD2AU+6m2WoU020bwGsFLBen4s=yg@mail.gmail.com>

On 09/12/2015 04:09 AM, Jonathan Wakely wrote:
> On 11 September 2015 at 18:39, Martin Sebor wrote:
>> On 09/11/2015 08:21 AM, Jonathan Wakely wrote:
>>>
>>> Solaris 10 doesn't follow POSIX in accepting a null pointer as the
>>> second argument to realpath(), so allocate a buffer for it.
>>
>>
>> FWIW, the NULL requirement is new in Issue 7. In Issue 6, the behavior
>> is implementation-defined, and before then it was an error. Solaris 10
>> claims conformance to SUSv2 and its realpath fails with EINVAL.
>> Solaris 11 says it conforms to Issue 6 but according to the man page
>> its realpath already implements the Issue 7 requirement.
>
> Thanks.
>
>> I suspect the same problem will come up on other systems so checking
>> the POSIX version might be better than testing for each OS.
>
> The problem with doing that is that many BSD systems have supported
> passing NULL as an extension long before issue 7, so if we just check
> something like _POSIX_VERSION >= 200809L then we can only canonicalize
> paths up to PATH_MAX on many systems where the extension is available
> but _POSIX_VERSION implies conformance to an older standard.

You're right. I agree that just checking the POSIX version may not
lead to optimal results. Some implementations also support multiple
versions and the one in effect may not be the one most recent. To
get the most out of those, it's usually recommended to set
_POSIX_C_SOURCE to the latest version before including any headers,
then test the supported version, and when the supported version is
less than the one requested and involves implementation defined
behavior (as in Issue 6) or undefined behavior that's known to be
used to provide extensions (as in SUSv2), check the implementation
version just as the patch does.

>
> So maybe we want an autoconf macro saying whether realpath() accepts
> NULL, and just hardcode it for the targets known to support it, and
> only check _POSIX_VERSION for the unknowns.

That will work for Issue 6 where the realpath behavior is
implementation-defined. The test wouldn't yield reliable results
for SUSv2 implementations where the behavior is undefined. There,
the result would have to be hardcoded based on what the manual says.
An autoconf test won't help with the ENAMETOOLONG problem since it
might depend on the filesystem. To overcome that, libstdc++ would
need to do the path traversal itself.

Martin

  reply	other threads:[~2015-09-12 18:07 UTC|newest]

Thread overview: 22+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2015-09-11 14:23 Jonathan Wakely
2015-09-11 18:05 ` Martin Sebor
2015-09-12 10:39   ` Jonathan Wakely
2015-09-12 19:49     ` Martin Sebor [this message]
2015-09-12 22:00       ` Martin Sebor
2015-09-16 14:52       ` Jonathan Wakely
2015-09-16 16:05         ` Jonathan Wakely
2015-09-16 16:11           ` Jonathan Wakely
2015-09-16 17:38         ` Martin Sebor
2015-09-16 19:02           ` Jonathan Wakely
2015-09-16 22:17             ` Martin Sebor
2015-09-16 22:23               ` Jonathan Wakely
2015-09-16 23:51                 ` Martin Sebor
2015-09-17 11:31                   ` Jonathan Wakely
2015-09-17 11:33                     ` Jonathan Wakely
2015-09-17 14:38                     ` Jonathan Wakely
2015-09-17 15:40                     ` Martin Sebor
2015-09-23 12:14                       ` Jonathan Wakely
2015-09-16 23:42             ` Jonathan Wakely
2015-09-17 15:36               ` Jonathan Wakely
2015-09-17 19:27             ` Andreas Schwab
2015-09-17 22:23               ` Jonathan Wakely

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=55F469CF.9010503@gmail.com \
    --to=msebor@gmail.com \
    --cc=gcc-patches@gcc.gnu.org \
    --cc=jwakely.gcc@gmail.com \
    --cc=jwakely@redhat.com \
    --cc=libstdc++@gcc.gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).