D17816: Support for xattrs on kio copy/move

[Kamil Dudka]

kdudka added a comment.


  I was wondering why copying files in Krusader or Dolphin from a vfat-formatted memory card stopped working for me after update.  After copying the first file, the transfer stopped progressing and the process ended up consuming 100% CPU forever.  Later I discovered that the cause of the breakage was this patch: If fgetxattr() fails with ENOTSUP, the code loops indefinitely calling fgetxattr().
  
  The following hotfix made copying of files in Krusader work again for me:
  
    --- a/src/ioslaves/file/file_unix.cpp
    +++ b/src/ioslaves/file/file_unix.cpp
    @@ -642,35 +642,35 @@ bool FileProtocol::copyXattrs(const int src_fd, const int dest_fd)
             ssize_t valuelen = 0;
             do {
                 value.resize(valuelen);
     #if HAVE_SYS_XATTR_H && !defined(__stub_getxattr) && !defined(Q_OS_MAC)
                 valuelen = fgetxattr(src_fd, key.constData(), value.data(), valuelen);
     #elif defined(Q_OS_MAC)
                 valuelen = fgetxattr(src_fd, key.constData(), value.data(), valuelen, 0, 0);
     #elif HAVE_SYS_EXTATTR_H
                 valuelen = extattr_get_fd(src_fd, EXTATTR_NAMESPACE_USER, key.constData(), valuelen == 0 ? nullptr : value.data(), valuelen);
     #endif
                 if (valuelen > 0 && value.size() == 0) {
                     continue;
                 }
                 if (valuelen > 0 && value.size() > 0) {
                     break;
                 }
    -            if (valuelen == -1 && errno == ERANGE) {
    +            if (valuelen == -1 && errno != ERANGE) {
                     valuelen = 0;
    -                continue;
    +                break;
                 }
                 // happens when attr value is an empty string
                 if (valuelen == 0) {
                     break;
                 }
             } while (true);
    
             // Write key:value pair on destination
     #if HAVE_SYS_XATTR_H && !defined(__stub_getxattr) && !defined(Q_OS_MAC)
             ssize_t destlen = fsetxattr(dest_fd, key.constData(), value.constData(), valuelen, 0);
     #elif defined(Q_OS_MAC)
             ssize_t destlen = fsetxattr(dest_fd, key.constData(), value.constData(), valuelen, 0, 0);
     #elif HAVE_SYS_EXTATTR_H
             ssize_t destlen = extattr_set_fd(dest_fd, EXTATTR_NAMESPACE_USER, key.constData(), value.constData(), valuelen);
     #endif
             if (destlen == -1 && errno == ENOTSUP) {
  
  However, the code might need bigger changes to cover all the cases.  I do not fully understand the (IMO over-complicated) loops around flistxattr() and fgetxattr().  Note that the fact that one of them uses `while (true) { ... }` whereas the other one uses `do { ... } while (true)` does not improve code readability either.

REVISION DETAIL
  https://phabricator.kde.org/D17816

To: arrowd, dfaure, chinmoyr, bruns, #frameworks, tmarshall, usta, cochise
Cc: kdudka, usta, scheirle, tmarshall, arrowd, cfeck, bruns, phidrho, dhaumann, funkybomber, abika, pino, davidedmundson, ngraham, atha.kane, spoorun, nicolasfella, kde-frameworks-devel, LeGast00n, cblack, michaelh
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/kde-frameworks-devel/attachments/20210302/764d7204/attachment-0001.htm>