This revision was automatically updated to reflect the committed changes.
Closed by commit R241:6b452ae9892d: Use non deprecated fastInsert in file.cpp
(first of many to come) (authored by jtamate).
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
dfaure accepted this revision.
dfaure added a comment.
This revision is now accepted and ready to land.
Thanks!
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D13898
To: jtamate, dfaure, #frameworks
Cc: bruns, kde-frameworks-devel, michaelh, ngraham
jtamate updated this revision to Diff 37427.
jtamate added a comment.
Use
st_birthtime
or
__st_birthtime
but not both.
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D13898?vs=37424=37427
REVISION DETAIL
dfaure added a comment.
The comments indicate that this isn't supposed to happen, but just to make
sure, you can use #elif on line 941/942.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D13898
To: jtamate, dfaure, #frameworks
Cc: bruns, kde-frameworks-devel, michaelh,
jtamate updated this revision to Diff 37424.
jtamate edited the summary of this revision.
jtamate added a comment.
Fixed targetPath of links.
Question: If there any chance that st_birthtime and __st_birthtime are both
present in the same OS? If affirmative, the entry of __st_birthtime
dfaure requested changes to this revision.
dfaure added inline comments.
This revision now requires changes to proceed.
INLINE COMMENTS
> bruns wrote in file.cpp:889
> isn't this the same as linkTargetBuffer?
Yes, and in fact linkTargetBuffer is more correct. toLocal8Bit() is incorrect,
bruns added inline comments.
INLINE COMMENTS
> file.cpp:889
> +#if HAVE_POSIX_ACL
> +targetPath = linkTarget.toLocal8Bit();
> +#endif
isn't this the same as linkTargetBuffer?
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D13898
To: jtamate, dfaure,
jtamate updated this revision to Diff 37240.
jtamate marked 2 inline comments as done.
jtamate added a comment.
Incorporated Stephan code.
Fill again the extra data like User/Group.
I don't see any change and I don't want to break anything more.
REPOSITORY
R241 KIO
CHANGES SINCE LAST
bruns added inline comments.
INLINE COMMENTS
> jtamate wrote in file.cpp:864
> According to the man page (2+3), readlink() //does not append a null byte to
> buf//.
>
> It will (silently) truncate the contents (to a length of bufsiz
> characters), in case the buffer is too small to hold all
jtamate added inline comments.
INLINE COMMENTS
> bruns wrote in file.cpp:864
> This is broken (although not new).
>
> `buff.st_size` is the size of the target name **without** null byte.
> readlink(..., .., bufferSize) thus will typically read exactly bufferSize
> bytes, thus n == bufferSize
>
jtamate updated this revision to Diff 37216.
jtamate marked 3 inline comments as done.
jtamate added a comment.
Added a comment: // readlink doesn't append a null byte to
linkTargetBuffer.data()
Using target path to read ACL in a non broken symbolic link.
Do not fill remaining data in
bruns added inline comments.
INLINE COMMENTS
> file.cpp:909
> * and it has a default ACL, also append that. */
> appendACLAtoms(path, entry, type);
> }
For UDS_ACCESS, _USER, _GROUP, we follow the symlink (i.e use the corresponding
buff), for ACL we do not?
>
bruns added inline comments.
INLINE COMMENTS
> file.cpp:864
> while (true) {
> ssize_t n = readlink(path.constData(),
> linkTargetBuffer.data(), bufferSize);
> if (n < 0 && errno != ERANGE) {
This is broken (although not new).
`buff.st_size` is
dfaure accepted this revision.
This revision is now accepted and ready to land.
REPOSITORY
R241 KIO
REVISION DETAIL
https://phabricator.kde.org/D13898
To: jtamate, dfaure, #frameworks
Cc: kde-frameworks-devel, michaelh, ngraham, bruns
jtamate updated this revision to Diff 37184.
jtamate edited the summary of this revision.
jtamate added a comment.
Renamed isSymLink to isBrokenSymLink.
REPOSITORY
R241 KIO
CHANGES SINCE LAST UPDATE
https://phabricator.kde.org/D13898?vs=37178=37184
REVISION DETAIL
dfaure added inline comments.
INLINE COMMENTS
> file.cpp:880
> // A symlink -> follow it only if details>1
> if (details > 1 && QT_STAT(path.constData(), ) == -1) {
> +isSymLink = true;
BTW the point is that we follow the link (by filling "buff" with
dfaure requested changes to this revision.
dfaure added a comment.
This revision now requires changes to proceed.
Yes I guess broken symlinks can have acl informations themselves too, so I
don't mind the acl code being called now and not before.
INLINE COMMENTS
> file.cpp:841
> mode_t
jtamate created this revision.
jtamate added reviewers: dfaure, Frameworks.
Restricted Application added a project: Frameworks.
Restricted Application added a subscriber: kde-frameworks-devel.
jtamate requested review of this revision.
REVISION SUMMARY
Avoid the goto using local variables.
18 matches
Mail list logo
