From a16a535ffd697aa2167eacf64e50ec3b74dd7aca Mon Sep 17 00:00:00 2001 From: Michael Andres Date: Mon, 28 Nov 2011 15:58:39 +0100 Subject: [PATCH] Fix and optimize Pathname ctor and provide testcases (bnc#721128) --- tests/zypp/CMakeLists.txt | 1 + tests/zypp/Pathname_test.cc | 151 +++++++++++++++++++++++++ zypp/Pathname.cc | 260 ++++++++++++++++++-------------------------- zypp/Pathname.h | 2 +- 4 files changed, 257 insertions(+), 157 deletions(-) create mode 100644 tests/zypp/Pathname_test.cc diff --git a/tests/zypp/CMakeLists.txt b/tests/zypp/CMakeLists.txt index 99dc71e..01862a5 100644 --- a/tests/zypp/CMakeLists.txt +++ b/tests/zypp/CMakeLists.txt @@ -21,6 +21,7 @@ ADD_TESTS( Locks MediaSetAccess PathInfo + Pathname PluginFrame PoolQuery ProgressData diff --git a/tests/zypp/Pathname_test.cc b/tests/zypp/Pathname_test.cc new file mode 100644 index 0000000..c749b4e --- /dev/null +++ b/tests/zypp/Pathname_test.cc @@ -0,0 +1,151 @@ +#include +#include + +#include + +#include "zypp/base/LogTools.h" +#include "zypp/Pathname.h" + +using boost::unit_test::test_suite; +using boost::unit_test::test_case; + +using namespace std; +using namespace zypp; + +BOOST_AUTO_TEST_CASE(pathname_default_ctor) +{ + Pathname p; + + BOOST_CHECK_EQUAL(p.empty(), true ); + BOOST_CHECK_EQUAL(p.absolute(), false ); + BOOST_CHECK_EQUAL(p.relative(), false ); + BOOST_CHECK_EQUAL(p.dirname(), "" ); + BOOST_CHECK_EQUAL(p.basename(), "" ); + BOOST_CHECK_EQUAL(p.extension(), "" ); + BOOST_CHECK_EQUAL(p.absolutename(), "" ); + BOOST_CHECK_EQUAL(p.relativename(), "" ); +} + +BOOST_AUTO_TEST_CASE(pathname_root) +{ + Pathname p("/"); + + BOOST_CHECK_EQUAL(p.empty(), false ); + BOOST_CHECK_EQUAL(p.absolute(), true ); + BOOST_CHECK_EQUAL(p.relative(), false ); + BOOST_CHECK_EQUAL(p.dirname(), "/" ); + BOOST_CHECK_EQUAL(p.basename(), "/" ); + BOOST_CHECK_EQUAL(p.extension(), "" ); + BOOST_CHECK_EQUAL(p.absolutename(), "/" ); + BOOST_CHECK_EQUAL(p.relativename(), "./" ); +} + +BOOST_AUTO_TEST_CASE(pathname_this) +{ + Pathname p("."); + + BOOST_CHECK_EQUAL(p.empty(), false ); + BOOST_CHECK_EQUAL(p.absolute(), false ); + BOOST_CHECK_EQUAL(p.relative(), true ); + BOOST_CHECK_EQUAL(p.dirname(), "." ); + BOOST_CHECK_EQUAL(p.basename(), "." ); + BOOST_CHECK_EQUAL(p.extension(), "" ); + BOOST_CHECK_EQUAL(p.absolutename(), "/" ); + BOOST_CHECK_EQUAL(p.relativename(), "." ); +} + +BOOST_AUTO_TEST_CASE(pathname_up) +{ + Pathname p(".."); + + BOOST_CHECK_EQUAL(p.empty(), false ); + BOOST_CHECK_EQUAL(p.absolute(), false ); + BOOST_CHECK_EQUAL(p.relative(), true ); + BOOST_CHECK_EQUAL(p.dirname(), "." ); + BOOST_CHECK_EQUAL(p.basename(), ".." ); + BOOST_CHECK_EQUAL(p.extension(), "" ); + BOOST_CHECK_EQUAL(p.absolutename(), "/" ); + BOOST_CHECK_EQUAL(p.relativename(), ".." ); +} + +BOOST_AUTO_TEST_CASE(pathname_abs) +{ + Pathname p("/foo/baa.ka"); + + BOOST_CHECK_EQUAL(p.empty(), false ); + BOOST_CHECK_EQUAL(p.absolute(), true ); + BOOST_CHECK_EQUAL(p.relative(), false ); + BOOST_CHECK_EQUAL(p.dirname(), "/foo" ); + BOOST_CHECK_EQUAL(p.basename(), "baa.ka" ); + BOOST_CHECK_EQUAL(p.extension(), ".ka" ); + BOOST_CHECK_EQUAL(p.absolutename(), "/foo/baa.ka" ); + BOOST_CHECK_EQUAL(p.relativename(), "./foo/baa.ka" ); +} + +BOOST_AUTO_TEST_CASE(pathname_rel) +{ + Pathname p("./foo/./../baa.ka"); + + BOOST_CHECK_EQUAL(p.empty(), false ); + BOOST_CHECK_EQUAL(p.absolute(), false ); + BOOST_CHECK_EQUAL(p.relative(), true ); + BOOST_CHECK_EQUAL(p.dirname(), "." ); + BOOST_CHECK_EQUAL(p.basename(), "baa.ka" ); + BOOST_CHECK_EQUAL(p.extension(), ".ka" ); + BOOST_CHECK_EQUAL(p.absolutename(), "/baa.ka" ); + BOOST_CHECK_EQUAL(p.relativename(), "./baa.ka" ); +} + +BOOST_AUTO_TEST_CASE(pathname_relup) +{ + Pathname p("./../foo/./../baa"); + + BOOST_CHECK_EQUAL(p.empty(), false ); + BOOST_CHECK_EQUAL(p.absolute(), false ); + BOOST_CHECK_EQUAL(p.relative(), true ); + BOOST_CHECK_EQUAL(p.dirname(), ".." ); + BOOST_CHECK_EQUAL(p.basename(), "baa" ); + BOOST_CHECK_EQUAL(p.extension(), "" ); + BOOST_CHECK_EQUAL(p.absolutename(), "/baa" ); + BOOST_CHECK_EQUAL(p.relativename(), "../baa" ); +} + +BOOST_AUTO_TEST_CASE(pathname_strval) +{ + BOOST_CHECK_EQUAL(Pathname("").asString(), "" ); + BOOST_CHECK_EQUAL(Pathname("/////./").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("./").asString(), "." ); + BOOST_CHECK_EQUAL(Pathname("/.").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("./..").asString(), "./.." ); // ? .. + BOOST_CHECK_EQUAL(Pathname("../").asString(), "./.." ); // ? .. + BOOST_CHECK_EQUAL(Pathname(".././..").asString(), "./../.." ); // ? ../.. + + + BOOST_CHECK_EQUAL(Pathname("//baa").asString(), "/baa" ); + BOOST_CHECK_EQUAL(Pathname("/./baa").asString(), "/baa" ); + BOOST_CHECK_EQUAL(Pathname("/baa/..").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("/baa/../baa").asString(), "/baa" ); + BOOST_CHECK_EQUAL(Pathname("/./../foo/./../baa").asString(), "/baa" ); + + BOOST_CHECK_EQUAL(Pathname("/").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname(".").asString(), "." ); + BOOST_CHECK_EQUAL(Pathname("..").asString(), "./.." ); + BOOST_CHECK_EQUAL(Pathname("/.").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("/..").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("/./.").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("/./..").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("/../.").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("/../..").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("/././").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("/./../").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("/.././").asString(), "/" ); + BOOST_CHECK_EQUAL(Pathname("/../../").asString(), "/" ); + + BOOST_CHECK_EQUAL(Pathname("a\\b").asString(), "./a\\b" ); + BOOST_CHECK_EQUAL(Pathname("a/b").asString(), "./a/b" ); + BOOST_CHECK_EQUAL(Pathname("c:a\\b").asString(), "./c:a\\b" ); + BOOST_CHECK_EQUAL(Pathname("c:a/b").asString(), "./c:a/b" ); + BOOST_CHECK_EQUAL(Pathname("cc:a\\b").asString(), "./cc:a\\b" ); + BOOST_CHECK_EQUAL(Pathname("cc:a/b").asString(), "./cc:a/b" ); +} + diff --git a/zypp/Pathname.cc b/zypp/Pathname.cc index 7b833f9..fb8037d 100644 --- a/zypp/Pathname.cc +++ b/zypp/Pathname.cc @@ -25,105 +25,6 @@ namespace zypp { ///////////////////////////////////////////////////////////////// /////////////////////////////////////////////////////////////////// - namespace - { ///////////////////////////////////////////////////////////////// - - /////////////////////////////////////////////////////////////////// - // - // CLASS NAME : DirStack - // - /** silly helper to build Pathnames. - */ - class DirStack { - - struct Dir { - - Dir * up; - Dir * dn; - string name; - - Dir( const string & n = "" ) { - name = n; - up = dn = 0; - } - - ~Dir() { - if ( up ) - up->dn = dn; - if ( dn ) - dn->up = up; - } - }; - - Dir * top; - Dir * bot; - - void Pop() { - if ( !top ) - return; - top = top->dn; - if ( top ) - delete top->up; - else { - delete bot; - bot = 0; - } - } - - public: - - DirStack() { top = bot = 0; } - ~DirStack() { - while ( bot ) - Pop(); - } - - void Push( const string & n ) { - if ( n.empty() || n == "." ) { // '.' or '/' only for bot - if ( bot ) - return; - } else if ( n == ".." && top ) { - if ( top->name == "" ) // "/.." ==> "/" - return; - - if ( top->name != "." && top->name != ".." ) { // "somedir/.." ==> "" - Pop(); - return; - } - // "../.." "./.." stays - } - - Dir * d = new Dir( n ); - if ( !top ) - top = bot = d; - else { - top->up = d; - d->dn = top; - d->up = 0; - top = d; - } - } - - string str() { - if ( !bot ) - return ""; - string ret; - for ( Dir * d = bot; d; d = d->up ) { - if ( d != bot ) - ret += "/"; - ret += d->name; - } - if ( ret.empty() ) - return "/"; - return ret; - } - }; - - ///////////////////////////////////////////////////////////////// - } // namespace - /////////////////////////////////////////////////////////////////// - - /////////////////////////////////////////////////////////////////// // // METHOD NAME : Pathname::_assign // METHOD TYPE : void @@ -131,55 +32,93 @@ namespace zypp void Pathname::_assign( const string & name_tv ) { prfx_i = 0; - name_t = name_tv; - - if ( name_t.empty() ) + name_t.clear(); + if ( name_tv.empty() ) return; + name_t.reserve( name_tv.size() ); + + // Collect up to "/.." + enum Pending { + P_none = 0, // "" + P_slash = 1, // "/" + P_dot1 = 2, // "/." + P_dot2 = 3 // "/.." + } pending = P_none; + + // Assert relative path starting with "./" + // We rely on this below! + if ( name_tv[0] != '/' ) + { + name_t += '.'; + pending = P_slash; + } - string Tprfx; - DirStack Stack_Ci; - - char * Buf_aci = new char[name_tv.length() + 1]; - char * W_pci = Buf_aci; - const char * R_pci = name_tv.c_str(); + // Lambda handling the "/.." case: + // [] + "/.." ==> [] + // [.] + "/.." ==> [./..] + // [foo] is always [./foo] due to init above + // [*/..] + "/.." ==> [*/../..] + // [*/foo] + "/.." ==> [*] + auto goParent_f = [&](){ + if ( name_t.empty() ) + /*NOOP*/; + else if ( name_t.size() == 1 ) // content is '.' + name_t += "/.."; + else + { + std::string::size_type pos = name_t.rfind( "/" ); + if ( pos == name_t.size() - 3 && name_t[pos+1] == '.' && name_t[pos+2] == '.' ) + name_t += "/.."; + else + name_t.erase( pos ); + } + }; - // check for prefix - if ( name_t.length() >= 2 - && name_t[1] == ':' - && ( ( 'a' <= name_t[0] && name_t[0] <= 'z' ) - || ( 'A' <= name_t[0] && name_t[0] <= 'Z' ) ) ) { - Tprfx = name_t.substr( 0, 2 ); - prfx_i = 2; - R_pci += 2; + for ( auto ch : name_tv ) + { + switch ( ch ) + { + case '/': + switch ( pending ) + { + case P_none: pending = P_slash; break; + case P_slash: break; + case P_dot1: pending = P_slash; break; + case P_dot2: goParent_f(); pending = P_slash; break; + } + break; + + case '.': + switch ( pending ) + { + case P_none: name_t += '.'; break; + case P_slash: pending = P_dot1; break; + case P_dot1: pending = P_dot2; break; + case P_dot2: name_t += "/..."; pending = P_none; break; + } + break; + + default: + switch ( pending ) + { + case P_none: break; + case P_slash: name_t += '/'; pending = P_none; break; + case P_dot1: name_t += "/."; pending = P_none; break; + case P_dot2: name_t += "/.."; pending = P_none; break; + } + name_t += ch; + break; + } } - // rel or abs path - if ( *R_pci == '/' ) { - Stack_Ci.Push( "" ); - ++R_pci; - } else { - Stack_Ci.Push( "." ); + switch ( pending ) + { + case P_none: break; + case P_slash: if ( name_t.empty() ) name_t = "/"; break; + case P_dot1: if ( name_t.empty() ) name_t = "/"; break; + case P_dot2: goParent_f(); if ( name_t.empty() ) name_t = "/"; break; } - - do { - switch ( *R_pci ) { - case '/': - case '\0': - if ( W_pci != Buf_aci ) { - *W_pci = '\0'; - W_pci = Buf_aci; - Stack_Ci.Push( Buf_aci ); - } - break; - - default: - *W_pci++ = *R_pci; - break; - } - } while( *R_pci++ ); - - delete[] Buf_aci; - name_t = Tprfx + Stack_Ci.str(); + return; } /////////////////////////////////////////////////////////////////// @@ -190,17 +129,15 @@ namespace zypp Pathname Pathname::dirname( const Pathname & name_tv ) { if ( name_tv.empty() ) - return ""; + return Pathname(); Pathname ret_t( name_tv ); string::size_type idx = ret_t.name_t.find_last_of( '/' ); if ( idx == string::npos ) { - ret_t.name_t.erase( ret_t.prfx_i ); - ret_t.name_t += "."; - } else if ( idx == ret_t.prfx_i ) { - ret_t.name_t.erase( ret_t.prfx_i ); - ret_t.name_t += "/"; + ret_t.name_t = "."; + } else if ( idx == 0 ) { + ret_t.name_t = "/"; } else { ret_t.name_t.erase( idx ); } @@ -219,9 +156,8 @@ namespace zypp return string(); string ret_t( name_tv.asString() ); - ret_t.erase( 0, name_tv.prfx_i ); string::size_type idx = ret_t.find_last_of( '/' ); - if ( idx != string::npos ) { + if ( idx != string::npos && ( idx != 0 || ret_t.size() != 1 ) ) { ret_t.erase( 0, idx+1 ); } @@ -264,8 +200,20 @@ namespace zypp string base( basename( name_tv ) ); string::size_type pos = base.rfind( '.' ); - if ( pos == string::npos ) - return string(); + switch ( pos ) + { + case 0: + if ( base.size() == 1 ) // . + return string(); + break; + case 1: + if ( base.size() == 2 && base[0] == '.' ) // .. + return string(); + break; + case string::npos: + return string(); + break; + } return base.substr( pos ); } @@ -295,10 +243,10 @@ namespace zypp if ( name_tv.empty() ) return add_tv; - string ret_ti( add_tv.asString() ); - ret_ti.replace( 0, add_tv.prfx_i, "/" ); - - return name_tv.asString() + ret_ti; + string ret_ti( name_tv.name_t ); + if( add_tv.name_t[0] != '/' ) + ret_ti += '/'; + return ret_ti + add_tv.name_t; } /////////////////////////////////////////////////////////////////// diff --git a/zypp/Pathname.h b/zypp/Pathname.h index a93c4b3..eedd7a5 100644 --- a/zypp/Pathname.h +++ b/zypp/Pathname.h @@ -40,7 +40,7 @@ namespace zypp * \todo Add support for handling extensions incl. stripping * extensions from basename (basename("/path/foo.baa", ".baa") ==> "foo") * \todo Review. Maybe use COW pimpl, check storage. - * \todo \b EXPLICIT ctors. + * \todo remove prfx_i */ class Pathname { -- 2.7.4