Review Request 112481: Revert port to QFile::Permissions
Aurélien Gâteau
agateau at kde.org
Tue Sep 3 12:59:39 UTC 2013
-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112481/
-----------------------------------------------------------
Review request for KDE Frameworks and David Faure.
Description
-------
Partly revert commit b03e81a61311ae1b64b0d37415477f9c08fe6142 which ported KFileItem to QFile::Permissions.
Quoting David Faure:
"Port back from QFile::Permissions to mode_t given that mode_t is available in
win32-msvc*/qplatformdefs.h (not everywhere, but in the places where the above
commits did a half job at porting from mode_t to QFile::Permissions).
QFile::Permissions is more limited (no suid bits) and having to convert between
mode_t and QFile::Permissions is just extra work."
I am not sure about the following part of KFileItemPrivate::init():
} else { // link pointing to nowhere (see kio/file/file.cc)
mode = (QT_STAT_MASK - 1) | S_IRWXU | S_IRWXG | S_IRWXO;
}
I blindly brought back from the old code without understanding it. If anything, the comment needs to be updated as there is no kio/file/file.cc anymore.
I am also wondering if this part of the code in isReadable() could be simplified a bit:
// No read permission at all
if (!(S_IRUSR & d->m_permissions) && !(S_IRGRP & d->m_permissions) && !(S_IROTH & d->m_permissions))
return false;
// Read permissions for all: save a stat call
if ((S_IRUSR|S_IRGRP|S_IROTH) & d->m_permissions)
return true;
Isn't this the same as this?
return (S_IRUSR|S_IRGRP|S_IROTH) & d->m_permissions;
Same thing for isWritable().
Diffs
-----
kio/tests/kfileitemtest.cpp c8b7a5d
staging/kio/src/core/kfileitem.h 623b426
staging/kio/src/core/kfileitem.cpp 970dea8
Diff: http://git.reviewboard.kde.org/r/112481/diff/
Testing
-------
No new unittest failure (5 kio tests are failing here, but they were already failing before the patch)
Thanks,
Aurélien Gâteau
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130903/1e12dcac/attachment.html>
More information about the Kde-frameworks-devel
mailing list