D17816: Support for xattrs on kio copy/move

[David Faure]

dfaure added a comment.


  Almost there ;)

INLINE COMMENTS

> file_unix.cpp:140
> +{
> +    // Get the size of the list of keys from soure file
> +#if HAVE_SYS_XATTR_H && !defined(__stub_getxattr) && !defined(Q_OS_MAC)

typo: soure => source

> file_unix.cpp:153
> +    if (listlen == 0) {
> +        qCDebug(KIO_FILE) << "file " << src_fd << " don't have any xattr";
> +        return false;

No need for spaces (after "file" and before "don't"), q[C]Debug inserts spaces automatically

> file_unix.cpp:166
> +#endif
> +    // Linux and MacOS return = list of null terminated string, each srting = [data,'\0']
> +    // BSD return = list of itens, each item prepended of 1 byte size = [size, data]

typo: srting => string

> file_unix.cpp:169
> +
> +    QByteArray::const_iterator offset = keylist.cbegin();
> +    size_t keyLen;

To me an offset is an integer that represents a relative index.

This is more like a source pointer, or keyData or something like that.

> file_unix.cpp:184
> +#if HAVE_SYS_XATTR_H
> +        key = qstrcpy(key.data(), offset);
> +#elif HAVE_SYS_EXTATTR_H

You're doing two copies here. From `offset` to `key.data()`, and then from `key.data()` -- the return value of qstrcpy -- into the QByteArray key (which calls the QByteArray(const char*) constructor).

This can be simplified.
Option 1: just remove the assignment, just do the qstrcpy. But it still smells like C to me. And there's a security bug if keyLen is ever too small.
Option 2: QByteArray key(offset); So simple. No need for strlen before, no need for qstrcpy, it all happens internally in that constructor.

> file_unix.cpp:186
> +#elif HAVE_SYS_EXTATTR_H
> +        key = qstrncpy(key.data(), offset, keyLen);
> +#endif

Same problem here. Similar solution: QByteArray key(offset, keyLen);

This removes the need for key.resize() before. Which also removes the need for the two parallel series of ifdefs, you can just merge them.

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/20200328/c26155a9/attachment-0001.html>