Review Request 112481: Revert port to QFile::Permissions

[Commit Hook]

-----------------------------------------------------------
This is an automatically generated e-mail. To reply, visit:
http://git.reviewboard.kde.org/r/112481/#review39276
-----------------------------------------------------------


This review has been submitted with commit 5581f3424563c53ac0288b5ad6265c8b7d130d4f by Aurélien Gâteau to branch frameworks.

- Commit Hook


On Sept. 3, 2013, 2: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, 2: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
> 
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20130903/38ec858c/attachment-0001.html>