On 01/05/15 20:38 +0100, Jonathan Wakely wrote:
On 01/05/15 21:28 +0200, Daniel Krügler wrote:
But if I read your implementation of path::compare(const path& p)
correctly it *also* may allocate memory by copying _M_pathname into a
_Cmpt object.
Yes, I agree that there's a bug here that could cause it to
std::terminate.
I was wondering whether for this comparison there exists
real need to *copy* _M_pathname (potentially exceeding the memory
limits). Wouldn't it be possible to define a _CmptRef type that for
the purpose of implementing compare(const path& p) just refers to
references of the _M_pathname and therefore doesn't need to allocate
any dynamic storage? (I should have spoken of a new type _CmptRef and
not of your existing _Cmpt).
Ah, I see what you mean (I thought you were suggesting some
improvements to _Cmpt itself ... which might be possible so I'm glad
you made me think about it :-)
I think I wrote compare() like that because it was easier, and when I
first implemented this we had COW strings so it wouldn't throw when
copying. That isn't true now, so I need to change it.
And here's the fix for this noexcept issue, it was very simple in the
end, thanks for the idea.
Tested powerpc64le-linux, committed to trunk.
commit 9c2f4abea2096bcc5e3568facf12fb3550358c6b
Author: Jonathan Wakely <jwak...@redhat.com>
Date: Fri May 1 20:57:21 2015 +0100
* src/filesystem/path.cc (path::compare): Do not copy strings.
diff --git a/libstdc++-v3/src/filesystem/path.cc b/libstdc++-v3/src/filesystem/path.cc
index cc5780f..7924732 100644
--- a/libstdc++-v3/src/filesystem/path.cc
+++ b/libstdc++-v3/src/filesystem/path.cc
@@ -107,17 +107,23 @@ namespace
int
path::compare(const path& p) const noexcept
{
+ struct CmptRef
+ {
+ const path* ptr;
+ const string_type& native() const noexcept { return ptr->native(); }
+ };
+
if (_M_type == _Type::_Multi && p._M_type == _Type::_Multi)
return do_compare(_M_cmpts.begin(), _M_cmpts.end(),
p._M_cmpts.begin(), p._M_cmpts.end());
else if (_M_type == _Type::_Multi)
{
- _Cmpt c[1] = { { p._M_pathname, p._M_type, 0 } };
+ CmptRef c[1] = { { &p } };
return do_compare(_M_cmpts.begin(), _M_cmpts.end(), c, c+1);
}
else if (p._M_type == _Type::_Multi)
{
- _Cmpt c[1] = { { _M_pathname, _M_type, 0 } };
+ CmptRef c[1] = { { this } };
return do_compare(c, c+1, p._M_cmpts.begin(), p._M_cmpts.end());
}
else
