----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: https://git.reviewboard.kde.org/r/124165/#review81846 -----------------------------------------------------------
(Hehe, that unittest should clearly use some ++int rather than all the hardcoded numbers, but OK, you don't have to do that) Just one last change request though. src/karchive.cpp (line 684) <https://git.reviewboard.kde.org/r/124165/#comment56179> The "/8 /8 %2" bit looks too arithmetic to me, this should be 'bitwise and' instead (faster, more approriate here, and IMHO more readable). We can't use S_IXUSR(perms), S_IXGRP(perms) and S_IXOTH(perms) due to windows portability, so I would suggest this instead: if (perm & 01) ret |= QFileDevice::ExeOther; if (perm & 010) ret |= QFileDevice::ExeGroup; if (perm & 0100) ret |= QFileDevice::ExeOwner; Also, QFileDevice::Permissions is a QFlags, so no casting needed. Declare QFileDevice::Permission ret = filePerms; and "return ret;" - David Faure On June 28, 2015, 10:31 a.m., Romário Rios wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > https://git.reviewboard.kde.org/r/124165/ > ----------------------------------------------------------- > > (Updated June 28, 2015, 10:31 a.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 > ----- > > autotests/karchivetest.cpp afc8387 > 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