In D22727#501824, @vonreth wrote:

Looks good.
Why not the first time 😊

I was afraid of reverts 😷

In D22727#505540, @dfaure wrote:

The Qt documentation says "Warning: Because of differences in the platforms supported by Qt, the semantics of ReadUser, WriteUser and ExeUser are platform-dependent: On Unix, the rights of the owner of the file are returned and on Windows the rights of the current user are returned. "

okay, I believe you mean I should leave these changes as-are in the diff. Am I right?

But, wait, this code is mixing "int permissions" (*) with the QFileDevice enum, that doesn't make any sense to me.

(*) this comes from KIO::put, which takes unix permissions on unix, not sure what it takes on Windows...

This doesn't match: unix permissions are octal (e.g. group read is 040 in octal), QFileDevice enum is hex (0x040).

Okay. I'm still unable to understand where/ how sftp::put() is called, or I would change everywhere it is called, to Qt way of permission extraction.

In D22727#510698, @brute4s99 wrote:

In D22727#505540, @dfaure wrote:

But, wait, this code is mixing "int permissions" (*) with the QFileDevice enum, that doesn't make any sense to me.

(*) this comes from KIO::put, which takes unix permissions on unix, not sure what it takes on Windows...

This doesn't match: unix permissions are octal (e.g. group read is 040 in octal), QFileDevice enum is hex (0x040).

Okay. I'm still unable to understand where/ how sftp::put() is called, or I would change everywhere it is called, to Qt way of permission extraction.

I would recommend not making API or ABI changes. It's annoying that they're not compatible, but you can probably find some way to convert from Qt permissions to the expected values in order to not change users

The truth is stronger than "I would not recommend".
put() in SlaveBase-derived classes is called by the KIO library (TransferJob), so you CANNOT change the meaning of the arguments. It's part of the API/ABI for all slaves, and this cannot change until KF6.

In D22727#513842, @dfaure wrote:

The truth is stronger than "I would not recommend".
put() in SlaveBase-derived classes is called by the KIO library (TransferJob), so you CANNOT change the meaning of the arguments. It's part of the API/ABI for all slaves, and this cannot change until KF6.

+1

But it is worth making a note of this as something to change (or discuss changing) for KF6 since it seems like using the Qt constants would be a better design

In D22727#513842, @dfaure wrote:

The truth is stronger than "I would not recommend".
put() in SlaveBase-derived classes is called by the KIO library (TransferJob), so you CANNOT change the meaning of the arguments. It's part of the API/ABI for all slaves, and this cannot change until KF6.

can you please add it to https://phabricator.kde.org/tag/kf6/ ?

I'm not sure what the QFileDevice enum gives us compared to the octal permissions as int? Is this just because it looks nicer, to save a conversion, or because you actually need the differenciation between ReadUser and ReadOwner, on Windows?

we need this because the symbols being currently used are Unix-only. Porting to Qt supplied ones allows this plugin to work on other OSes (my concern: Windows) as well. 😁

I suggest do look at kio_file (kio/src/ioslaves/file), the above code was inspired by kio_file, and then ported differently to Windows.