Fix and optimize Pathname ctor and provide testcases (bnc#721128)

diff --git a/tests/zypp/CMakeLists.txt b/tests/zypp/CMakeLists.txt
index 99dc71e..01862a5 100644 (file)
--- 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 (file)
index 0000000..c749b4e
--- /dev/null
+++ b/tests/zypp/Pathname_test.cc
@@ -0,0 +1,151 @@
+#include <iostream>
+#include <string>
+
+#include <boost/test/auto_unit_test.hpp>
+
+#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 (file)
--- 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 (file)
--- 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
     {