----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124165/#review81749 -----------------------------------------------------------
At this point I wonder if three if()s wouldn't be a lot less code :-) Anyway, more importantly: this is missing a unittest, you should extend testCopyTo() in karchivetest.cpp to include the case of an executable file. src/karchive.cpp (line 682) <https://git.reviewboard.kde.org/r/124165/#comment56057> static_cast<int>(filePerms). C casts make some nightmares come to mind again ;) src/karchive.cpp (line 684) <https://git.reviewboard.kde.org/r/124165/#comment56058> static src/karchive.cpp (line 687) <https://git.reviewboard.kde.org/r/124165/#comment56059> static src/karchive.cpp (line 697) <https://git.reviewboard.kde.org/r/124165/#comment56060> static_cast - David Faure On June 25, 2015, 12:44 p.m., Romário Rios wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124165/ > ----------------------------------------------------------- > > (Updated June 25, 2015, 12:44 p.m.) > > > Review request for KDE Frameworks. > > > Repository: karchive > > > Description > ------- > > The KArchiveFile::copyTo() doesn't set the permissions of the files it copies > -- it just copies the data. So if an archive contains an executable, it's > extracted from the archive as a non-executable. This patch makes it apply the > executable permissions found in the KArchiveFile to the QFile being copied. > > > Diffs > ----- > > src/karchive.cpp 344cba8 > > Diff: https://git.reviewboard.kde.org/r/124165/diff/ > > > Testing > ------- > > Seems to work for all files (tested with xz and zip files containing > executables, directories, text, etc.). > > > Thanks, > > Romário Rios > >
_______________________________________________ Kde-frameworks-devel mailing list Kde-frameworks-devel@kde.org https://mail.kde.org/mailman/listinfo/kde-frameworks-devel