D17816: Support for xattrs on kio copy/move

[Cochise César]

cochise added inline comments.

INLINE COMMENTS

> dfaure wrote in jobtest.cpp:118
> typo in Xatr, and in "foud". I suggest:
> 
>   qWarning() << "Neither getfattr, getextattr nor xattr was found";
> 
> Although I have to wonder why this looks for all three, to then only use getfattr and skip tests in all other cases....

The check is to build infrastructure for anyone with access to the platforms that want to add their test.
Not sure if this would ever happens, but it's here.

> 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)

Not because if the file don't have a xattr the function returns on 153. The returned keylist should have at least one key.
Removing last, if empy is done because in my tests the keylist includes a empty attribute that is preserved, and appended after each copy.

  if (listlen == 0) {
          qCDebug(KIO_FILE) << "file " << src_fd << " don't have any xattr";
          return false;
  }

> bruns wrote in file_unix.cpp:164
> 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
>   }

As the system functions need individual keys to get values and a individual key value pair to set, the list is to make more easy to iterate over keys. We can iterate over the buffer, reading until '\0', but this is a little more bug prone.

> bruns wrote in file_unix.cpp:164
> 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).**

Well... This will need some platform specif code to read the buffer and place in the m_keyList, ad the .split() will not work. Using the list is mandatory now, as we have different format buffers.
Committing other fixes, ad they are really small and postponed this one.

> bruns wrote in file_unix.cpp:180
> Allocate outside the loop

The value change on every pass and we only get the size inside the loop. Allocating outside the loop is more performant than resizing for each pass?

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/20200318/7ef0d4ec/attachment-0001.html>