From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 33753 invoked by alias); 16 Sep 2015 14:42:21 -0000 Mailing-List: contact gcc-patches-help@gcc.gnu.org; run by ezmlm Precedence: bulk List-Id: List-Archive: List-Post: List-Help: Sender: gcc-patches-owner@gcc.gnu.org Received: (qmail 33731 invoked by uid 89); 16 Sep 2015 14:42:20 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-1.3 required=5.0 tests=AWL,BAYES_00,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=no version=3.3.2 X-Spam-User: qpsmtpd, 2 recipients X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with (AES256-GCM-SHA384 encrypted) ESMTPS; Wed, 16 Sep 2015 14:42:10 +0000 Received: from int-mx13.intmail.prod.int.phx2.redhat.com (int-mx13.intmail.prod.int.phx2.redhat.com [10.5.11.26]) by mx1.redhat.com (Postfix) with ESMTPS id D333CC02C030; Wed, 16 Sep 2015 14:42:08 +0000 (UTC) Received: from localhost (ovpn-116-24.ams2.redhat.com [10.36.116.24]) by int-mx13.intmail.prod.int.phx2.redhat.com (8.14.4/8.14.4) with ESMTP id t8GEg7bX026158; Wed, 16 Sep 2015 10:42:08 -0400 Date: Wed, 16 Sep 2015 14:52:00 -0000 From: Jonathan Wakely To: Martin Sebor Cc: libstdc++ , gcc-patches Subject: Re: [patch] libstdc++/67173 Fix filesystem::canonical for Solaris 10. Message-ID: <20150916144207.GY2631@redhat.com> References: <20150911142140.GL2631@redhat.com> <55F311D2.8050405@gmail.com> <55F469CF.9010503@gmail.com> MIME-Version: 1.0 Content-Type: multipart/mixed; boundary="Dxzxec4+BSbG6TGA" Content-Disposition: inline In-Reply-To: <55F469CF.9010503@gmail.com> X-Clacks-Overhead: GNU Terry Pratchett User-Agent: Mutt/1.5.23 (2014-03-12) X-SW-Source: 2015-09/txt/msg01192.txt.bz2 --Dxzxec4+BSbG6TGA Content-Type: text/plain; charset=us-ascii; format=flowed Content-Disposition: inline Content-length: 4242 On 12/09/15 12:07 -0600, Martin Sebor wrote: >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. It turns out I was wrong about BSD traditionally supporting realpath(path, NULL), it first appeared in these relatively recent versions: FreeBSD 9.0 OpenBSD 5.0 NetBSD 6.1 Like Solaris 11, some of them still define _POSIX_VERSION=200112L even though they support passing NULL, so we could hardcode the targets that are known to support it, but we'll still need a fallback for lots of slightly older targets anyway. So here's a new implementation of canonical() which only uses realpath() when this autoconf compile-or-link test passes: #if _XOPEN_VERSION < 500 #error #elif _XOPEN_VERSION >= 700 || defined(PATH_MAX) char *tmp = realpath((const char*)NULL, (char*)NULL); #else #error #endif i.e. I use realpath() for Issue 7, or for Issue 6 if PATH_MAX is defined. Then in filesystem::canonical() if _GLIBCXX_USE_REALPATH is set I use it, passing NULL for Issue 7, or allocating a buffer of PATH_MAX otherwise. If realpath isn't supported or fails with ENAMETOOLONG then I do the path traversal by hand (which can be done entirely using the std::experimental::filesystem operations). Only passing NULL for Issue 7 is quite conservative. It means we don't do it for targets that support it as an implementation-defined extension to Issue 6, which includes Solaris 11, the BSDs and even older GNU systems (including RHEL6). But that's OK, we have a fallback now so it means no loss of functionality, just efficiency. We can tweak the config later for targets known to handle NULL. The new testcase is not very thorough. I've run a few more involved tests that aren't suitable to check in until I figure out a good way of running filesystem tests that can create/remove arbitrary files and symlinks. What do you think? Tested powerpc64le-linux and x86_64-dragonfly4.2. --Dxzxec4+BSbG6TGA Content-Type: text/x-patch; charset=us-ascii Content-Disposition: attachment; filename="patch.txt" Content-length: 7285 commit e79ad2dbcb14d1e66f6edead4ff87b62e575a8e7 Author: Jonathan Wakely Date: Wed Sep 16 14:07:54 2015 +0100 Implement filesystem::canonical() without realpath PR libstdc++/67173 * acinclude.m4 (GLIBCXX_CHECK_FILESYSTEM_DEPS): Check _XOPEN_VERSION and PATH_MAX for _GLIBCXX_USE_REALPATH. * config.h.in: Regenerate. * configure: Regenerate. * src/filesystem/dir.cc: Define _XOPEN_VERSION. * src/filesystem/ops.cc: Likewise. (canonical) [!_GLIBCXX_USE_REALPATH]: Add alternative implementation. * testsuite/experimental/filesystem/operations/canonical.cc: New. diff --git a/libstdc++-v3/acinclude.m4 b/libstdc++-v3/acinclude.m4 index 64c9b7e..364a7d2 100644 --- a/libstdc++-v3/acinclude.m4 +++ b/libstdc++-v3/acinclude.m4 @@ -3926,7 +3926,7 @@ dnl AC_LANG_SAVE AC_LANG_CPLUSPLUS ac_save_CXXFLAGS="$CXXFLAGS" - CXXFLAGS="$CXXFLAGS -fno-exceptions" + CXXFLAGS="$CXXFLAGS -fno-exceptions -D_XOPEN_SOURCE=700" dnl AC_MSG_CHECKING([for struct dirent.d_type]) AC_CACHE_VAL(glibcxx_cv_dirent_d_type, [dnl @@ -3947,13 +3947,24 @@ dnl AC_MSG_CHECKING([for realpath]) AC_CACHE_VAL(glibcxx_cv_realpath, [dnl GCC_TRY_COMPILE_OR_LINK( - [#include ], - [char *tmp = realpath((const char*)NULL, (char*)NULL);], + [ + #include + #include + ], + [ + #if _XOPEN_VERSION < 500 + #error + #elif _XOPEN_VERSION >= 700 || defined(PATH_MAX) + char *tmp = realpath((const char*)NULL, (char*)NULL); + #else + #error + #endif + ], [glibcxx_cv_realpath=yes], [glibcxx_cv_realpath=no]) ]) if test $glibcxx_cv_realpath = yes; then - AC_DEFINE(_GLIBCXX_USE_REALPATH, 1, [Define if realpath is available in .]) + AC_DEFINE(_GLIBCXX_USE_REALPATH, 1, [Define if usable realpath is available in .]) fi AC_MSG_RESULT($glibcxx_cv_realpath) dnl diff --git a/libstdc++-v3/src/filesystem/dir.cc b/libstdc++-v3/src/filesystem/dir.cc index 016a78d..cfa44b1 100644 --- a/libstdc++-v3/src/filesystem/dir.cc +++ b/libstdc++-v3/src/filesystem/dir.cc @@ -22,6 +22,8 @@ // see the files COPYING3 and COPYING.RUNTIME respectively. If not, see // . +#define _XOPEN_SOURCE 700 + #include #include #include diff --git a/libstdc++-v3/src/filesystem/ops.cc b/libstdc++-v3/src/filesystem/ops.cc index cefb927..45daf34 100644 --- a/libstdc++-v3/src/filesystem/ops.cc +++ b/libstdc++-v3/src/filesystem/ops.cc @@ -22,6 +22,8 @@ // see the files COPYING3 and COPYING.RUNTIME respectively. If not, see // . +#define _XOPEN_SOURCE 700 + #include #include #include @@ -96,23 +98,98 @@ namespace fs::path fs::canonical(const path& p, const path& base, error_code& ec) { - path can; + const path pa = absolute(p, base); + path result; #ifdef _GLIBCXX_USE_REALPATH - char* buffer = nullptr; -#if defined(__SunOS_5_10) && defined(PATH_MAX) - buffer = (char*)::malloc(PATH_MAX); -#endif - if (char_ptr rp = char_ptr{::realpath(absolute(p, base).c_str(), buffer)}) + char_ptr buf{ nullptr }; +# if _XOPEN_VERSION < 700 + // Not safe to call realpath(path, NULL) + buf.reset( (char*)::malloc(PATH_MAX) ); +# endif + if (char* rp = ::realpath(pa.c_str(), buf.get())) { - can.assign(rp.get()); + if (buf == nullptr) + buf.reset(rp); + result.assign(rp); ec.clear(); + return result; + } + if (errno != ENAMETOOLONG) + { + ec.assign(errno, std::generic_category()); + return result; } - else - ec.assign(errno, std::generic_category()); -#else - ec = std::make_error_code(std::errc::not_supported); #endif - return can; + + auto fail = [&ec, &result](int e) mutable { + if (!ec.value()) + ec.assign(e, std::generic_category()); + result.clear(); + }; + + if (!exists(pa, ec)) + { + fail(ENOENT); + return result; + } + // else we can assume no unresolvable symlink loops + + result = pa.root_path(); + + deque cmpts; + for (auto& f : pa.relative_path()) + cmpts.push_back(f); + + while (!cmpts.empty()) + { + path f = std::move(cmpts.front()); + cmpts.pop_front(); + + if (f.compare(".") == 0) + { + if (!is_directory(result, ec)) + { + fail(ENOTDIR); + break; + } + } + else if (f.compare("..") == 0) + { + auto parent = result.parent_path(); + if (parent.empty()) + result = pa.root_path(); + else + result.swap(parent); + } + else + { + result /= f; + + if (is_symlink(result, ec)) + { + path link = read_symlink(result, ec); + if (!ec.value()) + { + if (link.is_absolute()) + { + result = link.root_path(); + link = link.relative_path(); + } + else + result.remove_filename(); + + cmpts.insert(cmpts.begin(), link.begin(), link.end()); + } + } + + if (ec.value() || !exists(result, ec)) + { + fail(ENOENT); + break; + } + } + } + return result; } fs::path diff --git a/libstdc++-v3/testsuite/experimental/filesystem/operations/canonical.cc b/libstdc++-v3/testsuite/experimental/filesystem/operations/canonical.cc new file mode 100644 index 0000000..0bcb85b --- /dev/null +++ b/libstdc++-v3/testsuite/experimental/filesystem/operations/canonical.cc @@ -0,0 +1,74 @@ +// Copyright (C) 2015 Free Software Foundation, Inc. +// +// This file is part of the GNU ISO C++ Library. This library is free +// software; you can redistribute it and/or modify it under the +// terms of the GNU General Public License as published by the +// Free Software Foundation; either version 3, or (at your option) +// any later version. + +// This library is distributed in the hope that it will be useful, +// but WITHOUT ANY WARRANTY; without even the implied warranty of +// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the +// GNU General Public License for more details. + +// You should have received a copy of the GNU General Public License along +// with this library; see the file COPYING3. If not see +// . + +// { dg-options "-std=gnu++11 -lstdc++fs" } +// { dg-require-filesystem-ts "" } + +#include +#include +#include + +namespace fs = std::experimental::filesystem; + +void +test01() +{ + std::error_code ec; + fs::path p = ""; + canonical( p, ec ); + VERIFY( !ec ); + + canonical( __gnu_test::nonexistent_path(), ec ); + VERIFY( !ec ); + + p = "/"; + p = canonical( p, ec ); + VERIFY( p == "/" ); + VERIFY( ec ); + + p = "/."; + p = canonical( p, ec ); + VERIFY( p == "/" ); + VERIFY( ec ); + + p = "/.."; + p = canonical( p, ec ); + VERIFY( p == "/" ); + VERIFY( ec ); + + p = "/../.././."; + p = canonical( p, ec ); + VERIFY( p == "/" ); + VERIFY( ec ); + + p = "/dev/stdin"; + if (exists(p)) + { + auto p2 = canonical(p); + if (is_symlink(p)) + VERIFY( p != p2 ); + else + VERIFY( p == p2 ); + VERIFY( canonical(p2) == p2 ); + } +} + +int +main() +{ + test01(); +} --Dxzxec4+BSbG6TGA--