Differential D17816 Diff 83309 src/ioslaves/file/file_unix.cpp

Changeset View

Standalone View

View Options

src/ioslaves/file/file_unix.cpp

Show All 33 Lines
34#include <QThread> 34​#include <QThread>
35 35​
36#include <QDebug> 36​#include <QDebug>
37#include <kconfiggroup.h> 37​#include <kconfiggroup.h>
38#include <klocalizedstring.h> 38​#include <klocalizedstring.h>
39#include <kmountpoint.h> 39​#include <kmountpoint.h>
40 40​
41#include <errno.h> 41​#include <errno.h>
42#if HAVE_SYS_XATTR_H ​
43#include <sys/xattr.h> ​
44#include <stdint.h> 42​#include <stdint.h>
pinoUnsubmitted
Done

HAVE_SYS_XATTR_H from config-kioslave-file.h can be used here

pino: `HAVE_SYS_XATTR_H` from `config-kioslave-file.h` can be used here
pinoUnsubmitted
Done

better check for sys/extattr.h instead

pino: better check for `sys/extattr.h` instead
45#endif ​
46#include <utime.h> 43​#include <utime.h>
47 44​
48#include <KAuth> 45​#include <KAuth>
49#include <KRandom> 46​#include <KRandom>
50 47​
51#include "fdreceiver.h" 48​#include "fdreceiver.h"
52#include "statjob.h" 49​#include "statjob.h"
53 50​
54#if HAVE_STATX 51​#if HAVE_STATX
55#include <sys/stat.h> 52​#include <sys/stat.h>
56#include <sys/sysmacros.h> // for makedev() 53​#include <sys/sysmacros.h> // for makedev()
57#endif 54​#endif
58 55​
59#ifdef Q_OS_LINUX 56​#ifdef Q_OS_LINUX
60#include <sys/ioctl.h> 57​#include <sys/ioctl.h>
61#include <linux/fs.h> 58​#include <linux/fs.h>
62#endif 59​#endif
63 60​
64//sendfile has different semantics in different platforms 61​//sendfile has different semantics in different platforms
65#if HAVE_SENDFILE && defined Q_OS_LINUX 62​#if HAVE_SENDFILE && defined Q_OS_LINUX
66#define USE_SENDFILE 1 63​#define USE_SENDFILE 1
67#include <sys/sendfile.h> 64​#include <sys/sendfile.h>
68#endif 65​#endif
69 66​
67​#if HAVE_SYS_XATTR_H
68​#include <sys/xattr.h>
69​//BSD uses a different include
70​#elif HAVE_SYS_EXTATTR_H
71​#include <sys/extattr.h>
72​#endif
73​
70using namespace KIO; 74​using namespace KIO;
71 75​
72/* 512 kB */ 76​/* 512 kB */
73#define MAX_IPC_SIZE (1024*512) 77​#define MAX_IPC_SIZE (1024*512)
74 78​
75static bool 79​static bool
76same_inode(const QT_STATBUF &src, const QT_STATBUF &dest) 80​same_inode(const QT_STATBUF &src, const QT_STATBUF &dest)
77{ 81​{
▲ Show 20 Lines • Show All 458 Lines • ▼ Show 20 Line(s)539{
536 KAuth::Action execAction(QStringLiteral("org.kde.kio.file.exec")); 540​ KAuth::Action execAction(QStringLiteral("org.kde.kio.file.exec"));
537 execAction.setHelperId(QStringLiteral("org.kde.kio.file")); 541​ execAction.setHelperId(QStringLiteral("org.kde.kio.file"));
538 if (execAction.status() == KAuth::Action::AuthorizedStatus) { 542​ if (execAction.status() == KAuth::Action::AuthorizedStatus) {
539 return execWithElevatedPrivilege(action, args, errcode); 543​ return execWithElevatedPrivilege(action, args, errcode);
540 } 544​ }
541 return PrivilegeOperationReturnValue::failure(errcode); 545​ return PrivilegeOperationReturnValue::failure(errcode);
542} 546​}
543 547​
548​
549​bool FileProtocol::copyXattrs(const int src_fd, const int dest_fd)
dfaureUnsubmitted
Done

This whole method should be in #if HAVE_SYS_XATTR_H || HAVE_SYS_EXTATTR_H.

Otherwise, on systems with neither defined, a lot of compiler warnings will happen, like "keyLen isn't initialized" on line 591.

You can leave the method in the .h without #if (those defines aren't known there).
Here you can either #if the method itself (declared and not defined and not used, is OK, although some might say, a bit unclean)
or #if the whole method *contents*, but then in the #else you need Q_UNUSED(src_fd); Q_UNUSED(dest_fd); to avoid compiler warnings about unused parameters.

dfaure: This whole method should be in `#if HAVE_SYS_XATTR_H || HAVE_SYS_EXTATTR_H`. Otherwise, on…
550​{
551​ // Get the list of keys
dfaureUnsubmitted
Done

typo: soure => source

dfaure: typo: soure => source
552​ ssize_t listlen = 0;
brunsUnsubmitted
Done

The idea is almost right, the implementation is wrong.

1. set listlen to 0

2. resize keylist to listlen
3. execute flistxattr with size = keylist.size();
4a. if (3.) returns listlen > 0 and keylist.size() == 0, go to (2.)
4b. if (3.) returns listlen > 0 and keylist.size() > 0 -> break
4c. if (3.) returns listlen == -1 and errno == ERANGE, set listlen to 0 and go to (2.)
4d. if (3.) returns listlen == -1 and errno == ENOTSUP -> return
4e. if (3.) returns listlen == 0 -> return

5. resize keylist to listlen, the list may shrink between flistxattr calls.
bruns: The idea is almost right, the implementation is wrong. ``` 1. set listlen to 0 2. resize…
553​ QByteArray keylist;
brunsUnsubmitted
Done

just QByteArray keylist;

bruns: just `QByteArray keylist;`
554​ while (true) {
brunsUnsubmitted
Done

while (true) instead of do {} while(true)

bruns: `while (true)` instead of `do {} while(true)`
555​ keylist.resize(listlen);
556​#if HAVE_SYS_XATTR_H && !defined(__stub_getxattr) && !defined(Q_OS_MAC)
brunsUnsubmitted
Done

extattr_list_fd, here and everywhere else

bruns: extattr_list_**fd**, here and everywhere else
557​ listlen = flistxattr(src_fd, keylist.data(), listlen);
558​#elif defined(Q_OS_MAC)
559​ listlen = flistxattr(src_fd, keylist.data(), listlen, 0);
brunsUnsubmitted
Done

You never check if the source FS supports xattrs.

bruns: You never check if the source FS supports xattrs.
560​#elif HAVE_SYS_EXTATTR_H
brunsUnsubmitted
Done

Now try this when the source is e.g. FAT formatted ...

Make this dependent on errno.

bruns: Now try this when the source is e.g. FAT formatted ... Make this dependent on errno.
561​ listlen = extattr_list_fd(src_fd, EXTATTR_NAMESPACE_USER, keylist.data(), listlen);
562​#endif
563​ if (listlen > 0 && keylist.size() == 0) {
564​ continue;
dfaureUnsubmitted
Done

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

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

as listlen is -1 here, you always resize to 511 ...

bruns: as listlen is -1 here, you always resize to 511 ...
565​ }
566​ if (listlen > 0 && keylist.size() > 0) {
brunsUnsubmitted
Done

This is still definitely wrong ...

bruns: This is still definitely wrong ...
567​ break;
568​ }
569​ if (listlen == -1 && errno == ERANGE) {
570​ listlen = 0;
571​ continue;
572​ }
573​ if (listlen == -1) {
574​ if (errno == ENOTSUP) {
dfaureUnsubmitted
Done

Can this *ever* return an empty list, because keylist was empty?

(Then last() will assert on the next line)

dfaure: Can this *ever* return an empty list, because keylist was empty? (Then last() will assert on…
brunsUnsubmitted
Done

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
}
bruns: Why a temporary list at all? ``` off_t offset = 0; size_t keyLen; while (offset < keylist.size…
cochiseUnsubmitted
Done

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.

cochise: As the system functions need individual keys to get values and a individual key value pair to…
cochiseUnsubmitted
Done

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;
}
cochise: Not because if the file don't have a xattr the function returns on 153. The returned keylist…
brunsUnsubmitted
Done

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

bruns: This is wrong for the BSD implementation: > extattr_list_file() returns a list of attributes…
cochiseUnsubmitted
Done

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.

cochise: Well... This will need some platform specif code to read the buffer and place in the m_keyList…
brunsUnsubmitted
Done

man 2 flistxattr:

In addition, the errors documented in stat(2) can also occur.

So any error but ERANGE will exit the loop, and you will have bogus keylist contents.

Do return false on any error but ERANGE in the loop.

After the loop (i.e. when listlen is guaranteed to be > 0), resize keylist to listlen, the list may shrink.

bruns: man 2 flistxattr: > In addition, the errors documented in stat(2) can also occur. So any error…
575​ qCDebug(KIO_FILE) << "source filesystem does not support xattrs";
brunsUnsubmitted
Done

"does not"

bruns: "does not"
576​ }
577​ return false;
dfaureUnsubmitted
Done

typo: srting => string

dfaure: typo: srting => string
578​ }
ustaUnsubmitted
Done

small typo in comment : itens => items

usta: small typo in comment : itens => items
579​ if (listlen == 0) {
580​ qCDebug(KIO_FILE) << "the file don't have any xattr";
dfaureUnsubmitted
Done

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.

dfaure: To me an offset is an integer that represents a relative index. This is more like a source…
brunsUnsubmitted
Done

src_fd is quite meaningless as debug output

bruns: `src_fd` is quite meaningless as debug output
581​ return false;
582​ }
dfaureUnsubmitted
Done

What's the difference with QByteArray key; ?

dfaure: What's the difference with `QByteArray key;` ?
583​ }
brunsUnsubmitted
Done

Should be true

bruns: Should be `true`
584​
brunsUnsubmitted
Done

Thats bad. Thats really bad.

squeeze may reallocate. This invalidates keyPtr.

bruns: Thats bad. Thats really bad. squeeze may reallocate. This invalidates keyPtr.
585​ keylist.resize(listlen);
586​
brunsUnsubmitted
Done

why .squeeze() ?

bruns: why `.squeeze()` ?
587​ // Linux and MacOS return a list of null terminated strings, each string = [data,'\0']
588​ // BSDs return a list of items, each item consisting of the size byte
brunsUnsubmitted
Done

"return a list of null terminated strings"

bruns: "return a list of null terminated strings"
589​ // prepended to the key = [size, data]
brunsUnsubmitted
Done

"BSDs return a list of items, ..." or "BSD returns a list of items, ..."
"..., each item consisting of the size byte prepended to the key."

bruns: "BSDs return a list of items, ..." or "BSD returns a list of items, ..." "..., each item…
590​ QByteArray::const_iterator keyPtr = keylist.cbegin();
brunsUnsubmitted
Done

Allocate outside the loop

bruns: Allocate outside the loop
cochiseUnsubmitted
Done

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?

cochise: The value change on every pass and we only get the size inside the loop. Allocating outside the…
dfaureUnsubmitted
Done

Ah, I see, you still need to call strlen() yourself, for the code at the end of the iteration....

Well, then you might as well pass the value to the constructor so it doesn't need to do the same.

const QByteArray key(keyPtr, keyLen);

after the #endif, since it would now be the exact same line of code for both cases.

dfaure: Ah, I see, you still need to call `strlen()` yourself, for the code at the end of the iteration.
cochiseUnsubmitted
Done

This look really better.

cochise: This look really better.
591​ size_t keyLen;
brunsUnsubmitted
Done

*keyPtr needs cast to unsigned char.

bruns: `*keyPtr` needs cast to unsigned char.
592​ QByteArray value;
593​
594​ // For each key
dfaureUnsubmitted
Done

What's the purpose of these two lines modifying key, given that the next line assigns to key anyway?

dfaure: What's the purpose of these two lines modifying `key`, given that the next line assigns to…
cochiseUnsubmitted
Done

Removed clear, as is redundant.

cochise: Removed clear, as is redundant.
595​ while (keyPtr != keylist.cend()) {
brunsUnsubmitted
Done

valuelen

bruns: valuelen
dfaureUnsubmitted
Done

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.

dfaure: You're doing two copies here. From `offset` to `key.data()`, and then from `key.data()` -- the…
cochiseUnsubmitted
Done

So many time looking for a way to get a copy until a null, and it's in one of the constructors.
This looks a lot better.

cochise: So many time looking for a way to get a copy until a null, and it's in one of the constructors.
596​ // Get size of the key
brunsUnsubmitted
Done

"key size" or "size of the key"

bruns: "key size" or "size of the key"
597​#if HAVE_SYS_XATTR_H
brunsUnsubmitted
Done

valuelen

bruns: valuelen
dfaureUnsubmitted
Done

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.

dfaure: Same problem here. Similar solution: QByteArray key(offset, keyLen); This removes the need for…
brunsUnsubmitted
Not Done

QByteArray value should be created outside the keyPtr loop.

This reduces the calls to malloc for the QByteArray data storage.

Then for each attribute, do a value.resize(value.capacity).

bruns: `QByteArray value` should be created outside the keyPtr loop. This reduces the calls to malloc…
598​ keyLen = strlen(keyPtr);
599​#elif HAVE_SYS_EXTATTR_H
600​ keyLen = static_cast<unsigned char>(*keyPtr);
601​ keyPtr++;
602​#endif
603​ QByteArray key(keyPtr, keyLen);
604​
605​ // Get the value for key
606​ ssize_t valuelen = 0;
607​ do {
brunsUnsubmitted
Done

-1 + 512 = 511 ...

Should be value.resize(value.size() + 512) or something similar.

bruns: -1 + 512 = 511 ... Should be `value.resize(value.size() + 512)` or something similar.
608​ value.resize(valuelen);
609​#if HAVE_SYS_XATTR_H && !defined(__stub_getxattr) && !defined(Q_OS_MAC)
brunsUnsubmitted
Done

you can return here, no value in trying again and again ...

bruns: you can return here, no value in trying again and again ...
brunsUnsubmitted
Done

Again, valuelen may be -1 here ...

bruns: Again, valuelen may be -1 here ...
610​ valuelen = fgetxattr(src_fd, key.constData(), value.data(), valuelen);
611​#elif defined(Q_OS_MAC)
brunsUnsubmitted
Not Done

also wrong ...

bruns: also wrong ...
612​ valuelen = fgetxattr(src_fd, key.constData(), value.data(), valuelen, 0, 0);
613​#elif HAVE_SYS_EXTATTR_H
614​ valuelen = extattr_get_fd(src_fd, EXTATTR_NAMESPACE_USER, key.constData(), value.data(), valuelen);
615​#endif
616​ if (valuelen > 0 && value.size() == 0) {
617​ continue;
brunsUnsubmitted
Done

WRONG WRONG WRONG !!!

bruns: WRONG WRONG WRONG !!!
618​ }
619​ if (valuelen > 0 && value.size() > 0) {
620​ break;
brunsUnsubmitted
Done

No extra spaces for qDebug, inserts spaces itself.

bruns: No extra spaces for qDebug, inserts spaces itself.
621​ }
622​ if (valuelen == -1 && errno == ERANGE) {
623​ valuelen = 0;
624​ continue;
625​ }
brunsUnsubmitted
Done

Infinite loop on valuelen == 0

bruns: Infinite loop on valuelen == 0
arrowdAuthorUnsubmitted
Done

Why? ERANGE means we need to come up with new value for valuelen, so we set it to zero and start over. On the new iteration it gets passed into fgetxattr/extattr_get_fd to find out sufficient value.

arrowd: Why? `ERANGE` means we need to come up with new value for `valuelen`, so we set it to zero and…
brunsUnsubmitted
Done

"0" is a regular return value. Try your code with the following file:

touch t
setfattr -n user.foo -v "" t

and preferably add a test case ...

bruns: "0" is a regular return value. Try your code with the following file: ``` touch t setfattr -n…
626​ } while (true);
627​
628​ // Write key:value pair on destination
629​#if HAVE_SYS_XATTR_H && !defined(__stub_getxattr) && !defined(Q_OS_MAC)
630​ ssize_t destlen = fsetxattr(dest_fd, key.constData(), value.constData(), valuelen, 0);
631​#elif defined(Q_OS_MAC)
632​ ssize_t destlen = fsetxattr(dest_fd, key.constData(), value.constData(), valuelen, 0, 0);
633​#elif HAVE_SYS_EXTATTR_H
634​ ssize_t destlen = extattr_set_fd(dest_fd, EXTATTR_NAMESPACE_USER, key.constData(), value.constData(), valuelen);
635​#endif
636​ if (destlen == -1 && errno == ENOTSUP) {
637​ qCDebug(KIO_FILE) << "Destination filesystem does not support xattrs";
brunsUnsubmitted
Done

"does not"

bruns: "does not"
638​ return false;
639​ }
640​
641​#if HAVE_SYS_XATTR_H
642​ keyPtr += keyLen + 1;
643​#elif HAVE_SYS_EXTATTR_H
brunsUnsubmitted
Done

what about #elif defined(Q_OS_MAC)?

bruns: what about `#elif defined(Q_OS_MAC)`?
dfaureUnsubmitted
Done

@bruns HAVE_SYS_XATTR_H covers mac and non-mac here.

dfaure: @bruns HAVE_SYS_XATTR_H covers mac and non-mac here.
644​ keyPtr += keyLen;
645​#endif
646​ }
647​ return true;
648​}
649​
544void FileProtocol::copy(const QUrl &srcUrl, const QUrl &destUrl, 650​void FileProtocol::copy(const QUrl &srcUrl, const QUrl &destUrl,
545 int _mode, JobFlags _flags) 651​ int _mode, JobFlags _flags)
546{ 652​{
547 if (privilegeOperationUnitTestMode()) { 653​ if (privilegeOperationUnitTestMode()) {
548 finished(); 654​ finished();
549 return; 655​ return;
550 } 656​ }
551 657​
▲ Show 20 Lines • Show All 215 Lines • ▼ Show 20 Line(s)872#ifdef USE_SENDFILE
767 } 873​ }
768#endif 874​#endif
769 processedSize(processed_size); 875​ processedSize(processed_size);
770 } 876​ }
771#ifdef FICLONE 877​#ifdef FICLONE
772 } 878​ }
773#endif 879​#endif
774 880​
881​ // Copy Extended attributes
882​#if HAVE_SYS_XATTR_H || HAVE_SYS_EXTATTR_H
883​ if (!copyXattrs(src_file.handle(), dest_file.handle())) {
884​ qCDebug(KIO_FILE) << "cant copy Extended attributes";
885​ }
886​#endif
887​
775 src_file.close(); 888​ src_file.close();
776 dest_file.close(); 889​ dest_file.close();
777 890​
778 if (wasKilled()) { 891​ if (wasKilled()) {
779 qCDebug(KIO_FILE) << "Clean dest file after ioslave was killed:" << dest; 892​ qCDebug(KIO_FILE) << "Clean dest file after ioslave was killed:" << dest;
780 if (!QFile::remove(dest)) { // don't keep partly copied file 893​ if (!QFile::remove(dest)) { // don't keep partly copied file
781 execWithElevatedPrivilege(DEL, {_dest}, errno); 894​ execWithElevatedPrivilege(DEL, {_dest}, errno);
782 } 895​ }
▲ Show 20 Lines • Show All 98 Lines • ▼ Show 20 Line(s)
881} 994​}
882 995​
883#if HAVE_SYS_XATTR_H 996​#if HAVE_SYS_XATTR_H
884static bool isNtfsHidden(const QString &filename) 997​static bool isNtfsHidden(const QString &filename)
885{ 998​{
886 constexpr auto attrName = "system.ntfs_attrib_be"; 999​ constexpr auto attrName = "system.ntfs_attrib_be";
887 const auto filenameEncoded = QFile::encodeName(filename); 1000​ const auto filenameEncoded = QFile::encodeName(filename);
888 1001​
889 uint32_t intAttr = 0; 1002​ uint32_t intAttr = 0;
davidedmundsonUnsubmitted
Done

you need to check vallen for error returns.

davidedmundson: you need to check vallen for error returns.
890 constexpr size_t xattr_size = sizeof(intAttr); 1003​ constexpr size_t xattr_size = sizeof(intAttr);
891 char strAttr[xattr_size]; 1004​ char strAttr[xattr_size];
892#ifdef Q_OS_MACOS 1005​#ifdef Q_OS_MACOS
893 auto length = getxattr(filenameEncoded.data(), attrName, strAttr, xattr_size, 0, XATTR_NOFOLLOW); 1006​ auto length = getxattr(filenameEncoded.data(), attrName, strAttr, xattr_size, 0, XATTR_NOFOLLOW);
894#else 1007​#else
895 auto length = getxattr(filenameEncoded.data(), attrName, strAttr, xattr_size); 1008​ auto length = getxattr(filenameEncoded.data(), attrName, strAttr, xattr_size);
896#endif 1009​#endif
897 if (length <= 0) { 1010​ if (length <= 0) {
▲ Show 20 Lines • Show All 488 Lines • ▼ Show 20 Line(s)1495#if HAVE_POSIX_ACL
1386 // Empty strings mean leave as is 1499​ // Empty strings mean leave as is
1387 if (!ACLString.isEmpty()) { 1500​ if (!ACLString.isEmpty()) {
1388 acl_t acl = nullptr; 1501​ acl_t acl = nullptr;
1389 if (ACLString == QLatin1String("ACL_DELETE")) { 1502​ if (ACLString == QLatin1String("ACL_DELETE")) {
1390 // user told us to delete the extended ACL, so let's write only 1503​ // user told us to delete the extended ACL, so let's write only
1391 // the minimal (UNIX permission bits) part 1504​ // the minimal (UNIX permission bits) part
1392 acl = acl_from_mode(perm); 1505​ acl = acl_from_mode(perm);
1393 } 1506​ }
1394 acl = acl_from_text(ACLString.toLatin1().constData()); 1507​ acl = acl_from_text(ACLString.toLatin1().constData());
ustaUnsubmitted
Not Done

isnt this ignoring acl_from_mode part ? I mean not sure but i think we need to check if it is nullptr or not before assigning it otherwise we will ignore the acl_from_mode part.

usta: isnt this ignoring acl_from_mode part ? I mean not sure but i think we need to check if it is…
arrowdAuthorUnsubmitted
Not Done

You seem to be right. I blamed the code down to svn->git import and this bug was present all the time.

To properly fix this I'd start with a test for KIO::chmod job that changes ACL permissions, but I can't test this on FreeBSD as we don't have acl_from_mode.

Anyways, it is out of scope of this review.

arrowd: You seem to be right. I blamed the code down to svn->git import and this bug was present all…
1395 if (acl_valid(acl) == 0) { // let's be safe 1508​ if (acl_valid(acl) == 0) { // let's be safe
1396 ret = acl_set_file(path, ACL_TYPE_ACCESS, acl); 1509​ ret = acl_set_file(path, ACL_TYPE_ACCESS, acl);
1397 // qDebug() << "Set ACL on:" << path << "to:" << aclToText(acl); 1510​ // qDebug() << "Set ACL on:" << path << "to:" << aclToText(acl);
1398 } 1511​ }
1399 acl_free(acl); 1512​ acl_free(acl);
1400 if (ret != 0) { 1513​ if (ret != 0) {
1401 return ret; // better stop trying right away 1514​ return ret; // better stop trying right away
1402 } 1515​ }
Show All 22 Lines