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
