fix sftp links with new uds implementation
ClosedPublic

Authored by jtamate on Jun 11 2018, 8:03 AM.

Details

Summary

When listening a directory with links in sftp, the type of the entry of a symlink was inserted twice.
Now the entry data is inserted only once.
Get rid of the goto statement using local variables.

Test Plan

Tested in sftp://127.0.0.1 with broken symbolic links and sftp://127.0.0.1/usr/lib64 with a lot of symbolic links.

Diff Detail

Repository
R320 KIO Extras
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.
jtamate requested review of this revision.Jun 11 2018, 8:03 AM
jtamate created this revision.

Thanks for working on this! :-)

It is definitely better now - slave does not crash anymore and shows the links. However, I'd expect a link to a directory to behave like in filesystem browsing:

  • show proper folder-link icon (instead of a file link)
  • allow to enter the referenced folder

The slave crashes on a request to read a linked file. But I'm not sure if that worked before. I'll investigate.

jtamate updated this revision to Diff 36007.Jun 11 2018, 2:00 PM
jtamate edited the summary of this revision. (Show Details)

I misread what was after the goto. :-(
Modified also the similar method createUDSEntry.

martinkostolny accepted this revision.Jun 11 2018, 7:37 PM

Now it works perfectly, thanks! :)

This revision is now accepted and ready to land.Jun 11 2018, 7:37 PM
bruns added inline comments.Jun 11 2018, 8:58 PM
sftp/kio_sftp.cpp
407

There is still some inconsistent behaviour here:
If the file is a symlink and can be resolved, it uses user/group from the target. If it can not be resolved, it uses the details of the symlink.

424

superfluous newline

jtamate marked 2 inline comments as done.Jun 12 2018, 6:27 AM
jtamate added inline comments.
sftp/kio_sftp.cpp
407

I think it behaves as before:
If the file is not a broken symlink (sb2 != null), it uses sb=sb2 (as before the patch).
If the file is a broken symlink (sb2 == null), it uses the original sb (as before the patch).

424

This will vanish before I commit, I promise.

dfaure accepted this revision.Jun 12 2018, 7:24 AM
dfaure added inline comments.
sftp/kio_sftp.cpp
404

spurious space before KIO::

407

Bruns: that's on purpose, kio_file does the same (see QT_STAT() on the link target at file.cpp:878). Who cares about the size/permissions/user/group of a symlink? What's much more interesting is those attributes for the target.

This revision was automatically updated to reflect the committed changes.
jtamate marked 2 inline comments as done.

Should we backport this on the stable branch? Otherwise affected people will have to either downgrade kio or compile kio-extras from master...

Should we backport this on the stable branch? Otherwise affected people will have to either downgrade kio or compile kio-extras from master...

If the answer is yes, could someone do it? I'll not be available a few days.

I'm backporting it. Before I push: will this work also with previous KIO versions?

I'm backporting it. Before I push: will this work also with previous KIO versions?

Yes, it should. There is no new api in use.