[prev in list] [next in list] [prev in thread] [next in thread] 

List:       kde-commits
Subject:    Re: [kdelibs/KDE/4.10] kdecore/io: Do not overwrite directories with the same name inside an archive
From:       David Faure <faure () kde ! org>
Date:       2013-05-12 7:31:08
Message-ID: 6698959.7FFfx8eXjf () asterix
[Download RAW message or body]

On Sunday 12 May 2013 03:44:47 Dawit Alemayehu wrote:
> Git commit fa9fa9ce0271e44117a70f03af58aa1963ba5df5 by Dawit Alemayehu.
> Committed on 11/05/2013 at 21:38.
> Pushed by adawit into branch 'KDE/4.10'.
> 
> Do not overwrite directories with the same name inside an archive.
> 
> BUG: 206994
> REVIEW: 110392
> FIXED-IN: 4.10.4

While writing the test I saw that, before the fix, this warning appears:

 KArchiveDirectory::addEntry: directory  "/" has entry "d" already 

So why not just handle this situation within addEntry itself?
I.e. what about this patch? It works too, but I'm wondering why you didn't 
choose this solution:

diff --git a/kdecore/io/karchive.cpp b/kdecore/io/karchive.cpp
index 43539e3..88e1de0 100644
--- a/kdecore/io/karchive.cpp
+++ b/kdecore/io/karchive.cpp
@@ -762,6 +762,7 @@ void KArchiveDirectory::addEntry( KArchiveEntry* entry )
   if( d->entries.value( entry->name() ) ) {
       kWarning() << "directory " << name()
                   << "has entry" << entry->name() << "already";
+      return;
   }
   d->entries.insert( entry->name(), entry );
 }


-- 
David Faure, faure@kde.org, http://www.davidfaure.fr
Working on KDE, in particular KDE Frameworks 5

[prev in list] [next in list] [prev in thread] [next in thread]