D17816: Support for xattrs on kio copy/move

[Stefan Brüns]

bruns added inline comments.

INLINE COMMENTS

> jobtest.cpp:105
> +     ****/
> +    m_getXattrCmd = QStandardPaths::findExecutable("getfattr").split("/").last();
> +    if (m_getXattrCmd == "getfattr") {

Whats the reason to split off the path?
Also, you can not just concatenate the command and args, see https://doc.qt.io/qt-5/qprocess.html#details

  struct ProgramWithArgs { QString program; QStringList args; };
  QVector<QPair<ProgramWithArgs, ProgramWithArgs> xattrprograms = {
          { {"getfattr", {}},   {"setfattr", {}} },
          { {"getextattr", {}}, {"setextattr", {}} },
          { {"xattr", {"-p}},   {"xattr", {"-w"}} },
  };
  for (auto& getset : xattrprograms) {
      m_getXattrCmd.program = QStandardPaths::findExecutable(getset.first.program);
      m_setXattrCmd.program = QStandardPaths::findExecutable(getset.second.program);
      if (!m_getXattrCmd.programm.isEmpty() && !m_setXattrCmd.programm.isEmpty()) {
          m_getXattrCmd.args = getset.first.args;
          m_setXattrCmd.args = getset.second.args;
          break;
      }
  } 
  if (m_getXattrCmd.isEmpty() || m_getXattrCmd.isEmpty()) {
      qWarning() << "Neither getfattr, getextattr nor xattr was found.";
  }

> jobtest.cpp:512
> +        QVERIFY(xattrwriter.waitForStarted());
> +        QCOMPARE(xattrwriter.state(), QProcess::Running);
> +        QVERIFY(xattrwriter.waitForFinished(-1));

This seem bogus to me - what guarantees the process has not already finished?

> jobtest.cpp:524
> +
> +void JobTest::compareXattr(const QString &src, const QString &dest)
> +{

void JobTest::compareXattr(const QString &src, const QString &dest)
  {
      auto srcAttrs = readXattr(src);
      auto destAttrs = readXattr(dest);
      QCOMPARE(srcAttrs, destAttrs);
  }
  
  QList<QByteArray> JobTest::readXattr(const QString &path)
  {

> jobtest.cpp:536
> +    QVERIFY(xattrreader.waitForStarted());
> +    QCOMPARE(xattrreader.state(), QProcess::Running);
> +    QVERIFY(xattrreader.waitForFinished(-1));

dito

> file_unix.cpp:180
> +#elif HAVE_SYS_EXTATTR_H
> +        keyLen = *keyPtr;
> +        keyPtr++;

`*keyPtr` needs cast to unsigned char.

> file_unix.cpp:232
> +        keyPtr += keyLen;
> +#endif
> +    }

what about `#elif defined(Q_OS_MAC)`?

> file_unix.cpp:148
> +#endif
> +    if (listlen == -1) {
> +        qCDebug(KIO_FILE) << "libc failed to extract list of xattr from file";

You never check if the source FS supports xattrs.

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/20200329/65876c72/attachment.html>