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
