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