D10543: fix crashing with duplicate entries

[David Faure]

dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.


  Thanks for the fix!

INLINE COMMENTS

> karchive.cpp:471
>          } else {
> -            //qCWarning(KArchiveLog) << "Found" << path << "but it's not a directory";
> +            qCWarning(KArchiveLog) << "Found" << path << "but it's not a directory";
> +            if (ent->isFile()) {

Please remove this warning, it's ambiguous at this point what the problem is; better have only one clear warning once we know which case we're in.

> karchive.cpp:472
> +            qCWarning(KArchiveLog) << "Found" << path << "but it's not a directory";
> +            if (ent->isFile()) {
> +                const KArchiveFile *file = static_cast<const KArchiveFile *>(ent);

You can remove this if(), every entry is either a file or a directory, by design.
And the if() confused me, e.g. if it was false, the code below would still talk about an empty file ;). But it can't ever be false, so let's just remove that if().

> karchive.cpp:475
> +                if (file->size() > 0) {
> +                    qCWarning(KArchiveLog) << "It's a normal file, bailing out";
> +                    return nullptr;

Obviously that means mentioning "path" in this warning, something about that file being a duplicate.

> karchive.cpp:480
> +
> +            qCWarning(KArchiveLog) << "It's an empty file, assuming it is actually a directory and replacing";
> +            KArchiveEntry *myEntry = const_cast<KArchiveEntry*>(ent);

If this can happen, since there's no data loss, maybe a qCDebug is enough?
(this also needs to mention "path" of course)

> kzip.cpp:756
> +                    } else {
> +                        setErrorString(tr("File %1 is in folder %2, but %3 is actually a file.").arg(entryName).arg(path).arg(path));
> +                        delete entry;

Use multi-arg to avoid issues in case one of these strings contains %1.

  .arg(entryName, path, path)

REPOSITORY
  R243 KArchive

REVISION DETAIL
  https://phabricator.kde.org/D10543

To: sandsmark, dfaure, #frameworks
Cc: apol, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20180215/e0a00579/attachment.html>