> On Sept. 3, 2013, 4:35 p.m., David Faure wrote: > > It's now kioslave/file/file.cpp, please adjust indeed. > > > > Don't simplify isReadable: there's 000 (the first if), 444 (the second if), > > and other combinations (004, 040, 400) (the case where neither of these > > simple tests pass so we must use additional API to check for readability). > > You suggestion "a 4 anywhere" doesn't make the file necessarily readable to > > us (if we don't own it).
But then the second if should be "(S_IRUSR|S_IRGRP|S_IROTH) == d->m_permissions", not "(S_IRUSR|S_IRGRP|S_IROTH) & d->m_permissions", no? - Aurélien ----------------------------------------------------------- This is an automatically generated e-mail. To reply, visit: http://git.reviewboard.kde.org/r/112481/#review39266 ----------------------------------------------------------- On Sept. 3, 2013, 4:04 p.m., Aurélien Gâteau wrote: > > ----------------------------------------------------------- > This is an automatically generated e-mail. To reply, visit: > http://git.reviewboard.kde.org/r/112481/ > ----------------------------------------------------------- > > (Updated Sept. 3, 2013, 4:04 p.m.) > > > 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 > ------- > > All tests pass. > > > Thanks, > > Aurélien Gâteau > >
_______________________________________________ Kde-frameworks-devel mailing list [email protected] https://mail.kde.org/mailman/listinfo/kde-frameworks-devel
