D17816: Support for xattrs on kio copy/move

Stefan BrĂ¼ns noreply at phabricator.kde.org
Tue Mar 17 23:44:41 GMT 2020


bruns added inline comments.

INLINE COMMENTS

> file_unix.cpp:146
> +#elif HAVE_SYS_EXTATTR
> +    ssize_t listlen = extattr_list_file(src_fd, EXTATTR_NAMESPACE_USER, nullptr, 0);
> +#endif

extattr_list_**fd**, here and everywhere else

> file_unix.cpp:164
> +#endif
> +    QList<QByteArray> m_keyList = keylist.split('\0');
> +    if (m_keyList.last().isEmpty()) m_keyList.removeLast(); // the last item may be empty

This is wrong for the BSD implementation:

> extattr_list_file() returns a list	of attributes present in the requested
>  namespace.	 Each list entry consists of a **single byte containing the
>  length of the attribute name**, followed by the attribute name.  The	attri-
>  bute name is **not terminated by ASCII 0 (nul).**

> dfaure wrote in file_unix.cpp:164
> Can this *ever* return an empty list, because keylist was empty?
> 
> (Then last() will assert on the next line)

Why a temporary list at all?

  off_t offset = 0; size_t keyLen;
  while (offset < keylist.size()) {
  #if BSD
     keyLen = static_cast<unsigned char>(keylist[offset]);
     offset++;
  #elif LINUX
      keyLen = strlen(keylist[offset]
  #endif
      key = keylist.mid(offset, keyLen);
      /* copy */
  ...
  #if BSD
      offset += keyLen;
  #elif LINUX
      offset += keyLen + 1;
  #endif
  }

> file_unix.cpp:180
> +        }
> +        QByteArray value(valuelen + 1, Qt::Uninitialized);
> +        // get the value for key

Allocate outside the loop

> file_unix.cpp:185
> +#elif defined(Q_OS_MAC)
> +        vallen = fgetxattr(src_fd, key.constData(), value.data(), valuelen, 0, 0);
> +#elif HAVE_SYS_EXTATTR

valuelen

> file_unix.cpp:187
> +#elif HAVE_SYS_EXTATTR
> +        vallen = extattr_get_file(src_fd, EXTATTR_NAMESPACE_USER, key.constData(), value.data(), valuelen);
> +#endif

valuelen

> file_unix.cpp:199
> +            if (errno == ENOTSUP) {
> +                qCWarning(KIO_FILE) << "destination filesystem don't support xattrs";
> +            } else {

you can return here, no value in trying again and again ...

REPOSITORY
  R241 KIO

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

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


More information about the Kde-frameworks-devel mailing list