[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:       Dawit A <adawit () kde ! org>
Date:       2013-05-12 16:13:28
Message-ID: CALa28R6J=JRcOMsLyGQcNn_1LujHfqY=AVaKCdc9Cp+GhvDkGg () mail ! gmail ! com
[Download RAW message or body]

Did not realize that. I was so focused on debugging KTar through a debugger
to understand what is going on, I did not even bother to step into that
function. :( And obviously I did not even bother to check the debug output
either or I may have realized the fix was much simpler. Anyhow, I did not
choose this much much simpler solution because or ignorance. And yes that
works correctly for me as well. I will update the patch.


On Sun, May 12, 2013 at 3:31 AM, David Faure <faure@kde.org> wrote:

> 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
>
>

[Attachment #3 (text/html)]

<div dir="ltr">Did not realize that. I was so focused on debugging KTar through a \
debugger to understand what is going on, I did not even bother to step into that \
function. :( And obviously I did not even bother to check the debug output either or \
I may have realized the fix was much simpler. Anyhow, I did not choose this much much \
simpler solution because or ignorance. And yes that works correctly for me as well. I \
will update the patch.</div>

<div class="gmail_extra"><br><br><div class="gmail_quote">On Sun, May 12, 2013 at \
3:31 AM, David Faure <span dir="ltr">&lt;<a href="mailto:faure@kde.org" \
target="_blank">faure@kde.org</a>&gt;</span> wrote:<br><blockquote \
class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc \
solid;padding-left:1ex">

On Sunday 12 May 2013 03:44:47 Dawit Alemayehu wrote:<br>
&gt; Git commit fa9fa9ce0271e44117a70f03af58aa1963ba5df5 by Dawit Alemayehu.<br>
&gt; Committed on 11/05/2013 at 21:38.<br>
&gt; Pushed by adawit into branch &#39;KDE/4.10&#39;.<br>
&gt;<br>
&gt; Do not overwrite directories with the same name inside an archive.<br>
&gt;<br>
&gt; BUG: 206994<br>
&gt; REVIEW: 110392<br>
&gt; FIXED-IN: 4.10.4<br>
<br>
While writing the test I saw that, before the fix, this warning appears:<br>
<br>
  KArchiveDirectory::addEntry: directory   &quot;/&quot; has entry &quot;d&quot; \
already<br> <br>
So why not just handle this situation within addEntry itself?<br>
I.e. what about this patch? It works too, but I&#39;m wondering why you \
didn&#39;t<br> choose this solution:<br>
<br>
diff --git a/kdecore/io/karchive.cpp b/kdecore/io/karchive.cpp<br>
index 43539e3..88e1de0 100644<br>
--- a/kdecore/io/karchive.cpp<br>
+++ b/kdecore/io/karchive.cpp<br>
@@ -762,6 +762,7 @@ void KArchiveDirectory::addEntry( KArchiveEntry* entry )<br>
     if( d-&gt;entries.value( entry-&gt;name() ) ) {<br>
           kWarning() &lt;&lt; &quot;directory &quot; &lt;&lt; name()<br>
                             &lt;&lt; &quot;has entry&quot; &lt;&lt; entry-&gt;name() \
&lt;&lt; &quot;already&quot;;<br> +         return;<br>
     }<br>
     d-&gt;entries.insert( entry-&gt;name(), entry );<br>
  }<br>
<span class="HOEnZb"><font color="#888888"><br>
<br>
--<br>
David Faure, <a href="mailto:faure@kde.org">faure@kde.org</a>, <a \
href="http://www.davidfaure.fr" target="_blank">http://www.davidfaure.fr</a><br> \
Working on KDE, in particular KDE Frameworks 5<br> <br>
</font></span></blockquote></div><br></div>



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