Changeset View
Standalone View
sftp/kio_sftp.cpp
Show All 20 Lines | |||||
21 | 21 | | |||
22 | #include "kio_sftp.h" | 22 | #include "kio_sftp.h" | ||
23 | 23 | | |||
24 | #include <config-runtime.h> | 24 | #include <config-runtime.h> | ||
25 | #include "kio_sftp_debug.h" | 25 | #include "kio_sftp_debug.h" | ||
26 | #include "kio_sftp_trace_debug.h" | 26 | #include "kio_sftp_trace_debug.h" | ||
27 | #include <cerrno> | 27 | #include <cerrno> | ||
28 | #include <cstring> | 28 | #include <cstring> | ||
29 | #include <utime.h> | | |||
brute4s99: this builds without `utime.h` on my system (Arch Linux with latest Plasma, Qt and other… | |||||
30 | 29 | | |||
31 | #include <QCoreApplication> | 30 | #include <QCoreApplication> | ||
32 | #include <QDir> | 31 | #include <QDir> | ||
33 | #include <QFile> | 32 | #include <QFile> | ||
34 | #include <QVarLengthArray> | 33 | #include <QVarLengthArray> | ||
35 | #include <QMimeType> | 34 | #include <QMimeType> | ||
36 | #include <QMimeDatabase> | 35 | #include <QMimeDatabase> | ||
36 | #include <QDateTime> | ||||
37 | 37 | | |||
38 | #include <kuser.h> | 38 | #include <kuser.h> | ||
39 | #include <kmessagebox.h> | 39 | #include <kmessagebox.h> | ||
40 | 40 | | |||
41 | #include <klocalizedstring.h> | 41 | #include <klocalizedstring.h> | ||
42 | #include <kconfiggroup.h> | 42 | #include <kconfiggroup.h> | ||
43 | #include <kio/ioslave_defaults.h> | 43 | #include <kio/ioslave_defaults.h> | ||
44 | 44 | | |||
45 | #ifdef Q_OS_WIN | ||||
46 | #include <experimental/filesystem> // for permissions | ||||
47 | using namespace std::experimental::filesystem; | ||||
48 | #include <qplatformdefs.h> | ||||
49 | #else | ||||
50 | #include <utime.h> | ||||
51 | #endif | ||||
52 | | ||||
45 | #define KIO_SFTP_SPECIAL_TIMEOUT 30 | 53 | #define KIO_SFTP_SPECIAL_TIMEOUT 30 | ||
46 | 54 | | |||
47 | // How big should each data packet be? Definitely not bigger than 64kb or | 55 | // How big should each data packet be? Definitely not bigger than 64kb or | ||
48 | // you will overflow the 2 byte size variable in a sftp packet. | 56 | // you will overflow the 2 byte size variable in a sftp packet. | ||
49 | #define MAX_XFER_BUF_SIZE (60 * 1024) | 57 | #define MAX_XFER_BUF_SIZE (60 * 1024) | ||
50 | 58 | | |||
51 | #define KSFTP_ISDIR(sb) (sb->type == SSH_FILEXFER_TYPE_DIRECTORY) | 59 | #define KSFTP_ISDIR(sb) (sb->type == SSH_FILEXFER_TYPE_DIRECTORY) | ||
52 | 60 | | |||
▲ Show 20 Lines • Show All 197 Lines • ▼ Show 20 Line(s) | 257 | int sftpProtocol::authenticateKeyboardInteractive(AuthInfo &info) { | |||
250 | 258 | | |||
251 | int err = ssh_userauth_kbdint(mSession, nullptr, nullptr); | 259 | int err = ssh_userauth_kbdint(mSession, nullptr, nullptr); | ||
252 | 260 | | |||
253 | while (err == SSH_AUTH_INFO) { | 261 | while (err == SSH_AUTH_INFO) { | ||
254 | const QString name = QString::fromUtf8(ssh_userauth_kbdint_getname(mSession)); | 262 | const QString name = QString::fromUtf8(ssh_userauth_kbdint_getname(mSession)); | ||
255 | const QString instruction = QString::fromUtf8(ssh_userauth_kbdint_getinstruction(mSession)); | 263 | const QString instruction = QString::fromUtf8(ssh_userauth_kbdint_getinstruction(mSession)); | ||
256 | const int n = ssh_userauth_kbdint_getnprompts(mSession); | 264 | const int n = ssh_userauth_kbdint_getnprompts(mSession); | ||
257 | 265 | | |||
258 | qCDebug(KIO_SFTP_LOG) << "name=" << name << " instruction=" << instruction << " prompts=" << n; | 266 | qCDebug(KIO_SFTP_LOG) << "name=" << name << " instruction=" << instruction << " prompts=" << n; | ||
Use kdebugsettings to enable sftp debug output instead of qCDebug->qDebug (which adds a lot of noise to the review) dfaure: Use `kdebugsettings` to enable sftp debug output instead of qCDebug->qDebug (which adds a lot… | |||||
259 | 267 | | |||
260 | for (int i = 0; i < n; ++i) { | 268 | for (int i = 0; i < n; ++i) { | ||
261 | char echo; | 269 | char echo; | ||
262 | const char *answer = ""; | 270 | const char *answer = ""; | ||
263 | 271 | | |||
264 | const QString prompt = QString::fromUtf8(ssh_userauth_kbdint_getprompt(mSession, i, &echo)); | 272 | const QString prompt = QString::fromUtf8(ssh_userauth_kbdint_getprompt(mSession, i, &echo)); | ||
265 | qCDebug(KIO_SFTP_LOG) << "prompt=" << prompt << " echo=" << QString::number(echo); | 273 | qCDebug(KIO_SFTP_LOG) << "prompt=" << prompt << " echo=" << QString::number(echo); | ||
266 | if (echo) { | 274 | if (echo) { | ||
▲ Show 20 Lines • Show All 72 Lines • ▼ Show 20 Line(s) | 341 | void sftpProtocol::reportError(const QUrl &url, const int err) { | |||
339 | error(kioError, url.toDisplayString()); | 347 | error(kioError, url.toDisplayString()); | ||
340 | } | 348 | } | ||
341 | 349 | | |||
342 | bool sftpProtocol::createUDSEntry(const QString &filename, const QByteArray &path, | 350 | bool sftpProtocol::createUDSEntry(const QString &filename, const QByteArray &path, | ||
343 | UDSEntry &entry, short int details) { | 351 | UDSEntry &entry, short int details) { | ||
344 | mode_t access; | 352 | mode_t access; | ||
345 | char *link; | 353 | char *link; | ||
346 | bool isBrokenLink = false; | 354 | bool isBrokenLink = false; | ||
347 | long long fileType = S_IFREG; | 355 | long long fileType = QT_STAT_REG; | ||
348 | long long size = 0LL; | 356 | long long size = 0LL; | ||
349 | 357 | | |||
350 | Q_ASSERT(entry.count() == 0); | 358 | Q_ASSERT(entry.count() == 0); | ||
351 | 359 | | |||
352 | sftp_attributes sb = sftp_lstat(mSftp, path.constData()); | 360 | sftp_attributes sb = sftp_lstat(mSftp, path.constData()); | ||
353 | if (sb == nullptr) { | 361 | if (sb == nullptr) { | ||
354 | return false; | 362 | return false; | ||
355 | } | 363 | } | ||
Show All 17 Lines | 380 | } else { | |||
373 | sftp_attributes_free(sb); | 381 | sftp_attributes_free(sb); | ||
374 | sb = sb2; | 382 | sb = sb2; | ||
375 | } | 383 | } | ||
376 | } | 384 | } | ||
377 | } | 385 | } | ||
378 | 386 | | |||
379 | if (isBrokenLink) { | 387 | if (isBrokenLink) { | ||
380 | // It is a link pointing to nowhere | 388 | // It is a link pointing to nowhere | ||
381 | fileType = S_IFMT - 1; | 389 | fileType = QT_STAT_MASK - 1; | ||
390 | #ifdef Q_OS_WIN | ||||
391 | access = static_cast<mode_t>(perms::owner_all | perms::group_all | perms::others_all); | ||||
static_cast instead of c cast, a single cast should do. vonreth: static_cast instead of c cast, a single cast should do.
Are the types compatible? | |||||
392 | #else | ||||
382 | access = S_IRWXU | S_IRWXG | S_IRWXO; | 393 | access = S_IRWXU | S_IRWXG | S_IRWXO; | ||
394 | #endif | ||||
383 | size = 0LL; | 395 | size = 0LL; | ||
384 | } else { | 396 | } else { | ||
385 | switch (sb->type) { | 397 | switch (sb->type) { | ||
386 | case SSH_FILEXFER_TYPE_REGULAR: | 398 | case SSH_FILEXFER_TYPE_REGULAR: | ||
387 | fileType = S_IFREG; | 399 | fileType = QT_STAT_REG; | ||
388 | break; | 400 | break; | ||
389 | case SSH_FILEXFER_TYPE_DIRECTORY: | 401 | case SSH_FILEXFER_TYPE_DIRECTORY: | ||
390 | fileType = S_IFDIR; | 402 | fileType = QT_STAT_DIR; | ||
391 | break; | 403 | break; | ||
392 | case SSH_FILEXFER_TYPE_SYMLINK: | 404 | case SSH_FILEXFER_TYPE_SYMLINK: | ||
393 | fileType = S_IFLNK; | 405 | fileType = QT_STAT_LNK; | ||
394 | break; | 406 | break; | ||
vonreth: What about the QFileInfo? | |||||
For my use case, I could not find any possibility of symlinks, (traversing Android filesystem) so I left it blank in here for any future dev to fix it. brute4s99: For my use case, I could not find any possibility of symlinks, (traversing Android filesystem)… | |||||
Yes, QT_STAT_LNK works on Unix too, we use it in many places in KIO So I'm pretty sure you can remove this ifdef. You could also replace S_IFREG with QT_STAT_REG and S_IFDIR with QT_STAT_DIR, for consistency. dfaure: Yes, QT_STAT_LNK works on Unix too, we use it in many places in KIO
(and it has the value… | |||||
395 | case SSH_FILEXFER_TYPE_SPECIAL: | 407 | case SSH_FILEXFER_TYPE_SPECIAL: | ||
396 | case SSH_FILEXFER_TYPE_UNKNOWN: | 408 | case SSH_FILEXFER_TYPE_UNKNOWN: | ||
397 | fileType = S_IFMT - 1; | 409 | fileType = QT_STAT_MASK - 1; | ||
398 | break; | 410 | break; | ||
399 | } | 411 | } | ||
400 | access = sb->permissions & 07777; | 412 | access = sb->permissions & 07777; | ||
401 | size = sb->size; | 413 | size = sb->size; | ||
402 | } | 414 | } | ||
403 | entry.fastInsert(KIO::UDSEntry::UDS_FILE_TYPE, fileType); | 415 | entry.fastInsert(KIO::UDSEntry::UDS_FILE_TYPE, fileType); | ||
404 | entry.fastInsert(KIO::UDSEntry::UDS_ACCESS, access); | 416 | entry.fastInsert(KIO::UDSEntry::UDS_ACCESS, access); | ||
405 | entry.fastInsert(KIO::UDSEntry::UDS_SIZE, size); | 417 | entry.fastInsert(KIO::UDSEntry::UDS_SIZE, size); | ||
▲ Show 20 Lines • Show All 1340 Lines • ▼ Show 20 Line(s) | 1756 | if (fstat) { | |||
1746 | totalBytesSent += fstat->size; | 1758 | totalBytesSent += fstat->size; | ||
1747 | sftp_attributes_free(fstat); | 1759 | sftp_attributes_free(fstat); | ||
1748 | } | 1760 | } | ||
1749 | } | 1761 | } | ||
1750 | } else { | 1762 | } else { | ||
1751 | mode_t initialMode; | 1763 | mode_t initialMode; | ||
1752 | 1764 | | |||
1753 | if (permissions != -1) { | 1765 | if (permissions != -1) { | ||
1766 | #ifdef Q_OS_WIN | ||||
1767 | initialMode = permissions | static_cast<mode_t>(perms::owner_write | perms::owner_read); | ||||
vonreth: single static cast as above | |||||
1768 | #else | ||||
1754 | initialMode = permissions | S_IWUSR | S_IRUSR; | 1769 | initialMode = permissions | S_IWUSR | S_IRUSR; | ||
1770 | #endif | ||||
1755 | } else { | 1771 | } else { | ||
1756 | initialMode = 0644; | 1772 | initialMode = 0644; | ||
1757 | } | 1773 | } | ||
1758 | 1774 | | |||
1759 | qCDebug(KIO_SFTP_LOG) << "Trying to open:" << QString(dest) << ", mode=" << QString::number(initialMode); | 1775 | qCDebug(KIO_SFTP_LOG) << "Trying to open:" << QString(dest) << ", mode=" << QString::number(initialMode); | ||
1760 | file = sftp_open(mSftp, dest.constData(), O_CREAT | O_TRUNC | O_WRONLY, initialMode); | 1776 | file = sftp_open(mSftp, dest.constData(), O_CREAT | O_TRUNC | O_WRONLY, initialMode); | ||
1761 | } // flags & KIO::Resume | 1777 | } // flags & KIO::Resume | ||
1762 | 1778 | | |||
▲ Show 20 Lines • Show All 153 Lines • ▼ Show 20 Line(s) | 1898 | { | |||
1916 | 1932 | | |||
1917 | finished(); | 1933 | finished(); | ||
1918 | } | 1934 | } | ||
1919 | 1935 | | |||
1920 | sftpProtocol::StatusCode sftpProtocol::sftpCopyGet(const QUrl& url, const QString& sCopyFile, int permissions, KIO::JobFlags flags, int& errorCode) | 1936 | sftpProtocol::StatusCode sftpProtocol::sftpCopyGet(const QUrl& url, const QString& sCopyFile, int permissions, KIO::JobFlags flags, int& errorCode) | ||
1921 | { | 1937 | { | ||
1922 | qCDebug(KIO_SFTP_LOG) << url << "->" << sCopyFile << ", permissions=" << permissions; | 1938 | qCDebug(KIO_SFTP_LOG) << url << "->" << sCopyFile << ", permissions=" << permissions; | ||
1923 | 1939 | | |||
1924 | // check if destination is ok ... | 1940 | // check if destination is ok ... | ||
1925 | QT_STATBUF buff; | 1941 | QFileInfo copyFile(sCopyFile); | ||
We're now doing stat() twice, once here, and once in QFileInfo just below. This could be fixed by using QFileInfo for everything: const bool bDestExists = info.exists(); dfaure: We're now doing stat() twice, once here, and once in QFileInfo just below.
This could be fixed… | |||||
Please remove this variable, since you're not calling QT_STAT anymore. You'll notice that you're still using buff.st_size, which should become QFileInfo's size() method instead... dfaure: Please remove this variable, since you're not calling QT_STAT anymore.
You'll notice that… | |||||
1926 | const bool bDestExists = (QT_STAT(QFile::encodeName(sCopyFile), &buff) != -1); | 1942 | const bool bDestExists = copyFile.exists(); | ||
1927 | | ||||
1928 | if(bDestExists) { | 1943 | if(bDestExists) { | ||
1929 | if(S_ISDIR(buff.st_mode)) { | 1944 | if(copyFile.isDir()) { | ||
1930 | errorCode = ERR_IS_DIRECTORY; | 1945 | errorCode = ERR_IS_DIRECTORY; | ||
1931 | return sftpProtocol::ClientError; | 1946 | return sftpProtocol::ClientError; | ||
1932 | } | 1947 | } | ||
1933 | 1948 | | |||
1934 | if(!(flags & KIO::Overwrite)) { | 1949 | if(!(flags & KIO::Overwrite)) { | ||
1935 | errorCode = ERR_FILE_ALREADY_EXIST; | 1950 | errorCode = ERR_FILE_ALREADY_EXIST; | ||
1936 | return sftpProtocol::ClientError; | 1951 | return sftpProtocol::ClientError; | ||
1937 | } | 1952 | } | ||
1938 | } | 1953 | } | ||
1939 | 1954 | | |||
1940 | bool bResume = false; | 1955 | bool bResume = false; | ||
1941 | const QString sPart = sCopyFile + QLatin1String(".part"); // do we have a ".part" file? | 1956 | const QString sPart = sCopyFile + QLatin1String(".part"); // do we have a ".part" file? | ||
1942 | const bool bPartExists = (QT_STAT(QFile::encodeName(sPart), &buff) != -1); | 1957 | QFileInfo partFile(sPart); | ||
1958 | const bool bPartExists = partFile.exists(); | ||||
1943 | const bool bMarkPartial = config()->readEntry("MarkPartial", true); | 1959 | const bool bMarkPartial = config()->readEntry("MarkPartial", true); | ||
1944 | const QString dest = (bMarkPartial ? sPart : sCopyFile); | 1960 | const QString dest = (bMarkPartial ? sPart : sCopyFile); | ||
1945 | 1961 | | |||
1946 | if (bMarkPartial && bPartExists && buff.st_size > 0) { | 1962 | if (bMarkPartial && bPartExists && copyFile.size() > 0) { | ||
1947 | if (S_ISDIR(buff.st_mode)) { | 1963 | if(partFile.isDir()) { | ||
WRONG. Here we were testing the result of stat() on the .part file, see old line 1942. Granted, reusing "buff" didn't make the code very readable.... Create a different QFileInfo instance for the .part file, and use it for bPartExists and for isDir() here. dfaure: WRONG. Here we were testing the result of stat() on the .part file, see old line 1942.
Granted… | |||||
1948 | errorCode = ERR_DIR_ALREADY_EXIST; | 1964 | errorCode = ERR_DIR_ALREADY_EXIST; | ||
1949 | return sftpProtocol::ClientError; // client side error | 1965 | return sftpProtocol::ClientError; // client side error | ||
1950 | } | 1966 | } | ||
1951 | bResume = canResume( buff.st_size ); | 1967 | bResume = canResume( copyFile.size() ); | ||
dfaure: Red alert! Red alert! Uninitialized data being used! | |||||
1952 | } | 1968 | } | ||
1953 | 1969 | | |||
1954 | if (bPartExists && !bResume) // get rid of an unwanted ".part" file | 1970 | if (bPartExists && !bResume) // get rid of an unwanted ".part" file | ||
1955 | QFile::remove(sPart); | 1971 | QFile::remove(sPart); | ||
1956 | 1972 | | |||
1957 | // WABA: Make sure that we keep writing permissions ourselves, | 1973 | // WABA: Make sure that we keep writing permissions ourselves, | ||
1958 | // otherwise we can be in for a surprise on NFS. | 1974 | // otherwise we can be in for a surprise on NFS. | ||
1959 | mode_t initialMode; | 1975 | mode_t initialMode; | ||
1960 | if (permissions != -1) | 1976 | if (permissions != -1) | ||
1977 | #ifdef Q_OS_WIN | ||||
1978 | initialMode = permissions | static_cast<mode_t>(perms::owner_write); | ||||
vonreth: cast | |||||
1979 | #else | ||||
1961 | initialMode = permissions | S_IWUSR; | 1980 | initialMode = permissions | S_IWUSR; | ||
1981 | #endif | ||||
1962 | else | 1982 | else | ||
1963 | initialMode = 0666; | 1983 | initialMode = 0666; | ||
1964 | 1984 | | |||
1965 | // open the output file ... | 1985 | // open the output file ... | ||
1966 | int fd = -1; | 1986 | int fd = -1; | ||
1967 | KIO::fileoffset_t offset = 0; | 1987 | KIO::fileoffset_t offset = 0; | ||
1968 | if (bResume) { | 1988 | if (bResume) { | ||
1969 | fd = QT_OPEN( QFile::encodeName(sPart), O_RDWR ); // append if resuming | 1989 | fd = QT_OPEN( QFile::encodeName(sPart), O_RDWR ); // append if resuming | ||
Show All 19 Lines | |||||
1989 | 2009 | | |||
1990 | if( ::close(fd) && result == sftpProtocol::Success ) { | 2010 | if( ::close(fd) && result == sftpProtocol::Success ) { | ||
1991 | errorCode = ERR_COULD_NOT_WRITE; | 2011 | errorCode = ERR_COULD_NOT_WRITE; | ||
1992 | result = sftpProtocol::ClientError; | 2012 | result = sftpProtocol::ClientError; | ||
1993 | } | 2013 | } | ||
1994 | 2014 | | |||
1995 | // handle renaming or deletion of a partial file ... | 2015 | // handle renaming or deletion of a partial file ... | ||
1996 | if (bMarkPartial) { | 2016 | if (bMarkPartial) { | ||
2017 | partFile.refresh(); | ||||
1997 | if (result == sftpProtocol::Success) { // rename ".part" on success | 2018 | if (result == sftpProtocol::Success) { // rename ".part" on success | ||
1998 | if ( !QFile::rename( QFile::encodeName(sPart), sCopyFile ) ) { | 2019 | if ( !QFile::rename( QFile::encodeName(sPart), sCopyFile ) ) { | ||
1999 | // If rename fails, try removing the destination first if it exists. | 2020 | // If rename fails, try removing the destination first if it exists. | ||
2000 | if (!bDestExists || !QFile::remove(sCopyFile) || !QFile::rename(sPart, sCopyFile)) { | 2021 | if (!bDestExists || !QFile::remove(sCopyFile) || !QFile::rename(sPart, sCopyFile)) { | ||
2001 | qCDebug(KIO_SFTP_LOG) << "cannot rename " << sPart << " to " << sCopyFile; | 2022 | qCDebug(KIO_SFTP_LOG) << "cannot rename " << sPart << " to " << sCopyFile; | ||
2002 | errorCode = ERR_CANNOT_RENAME_PARTIAL; | 2023 | errorCode = ERR_CANNOT_RENAME_PARTIAL; | ||
2003 | result = sftpProtocol::ClientError; | 2024 | result = sftpProtocol::ClientError; | ||
2004 | } | 2025 | } | ||
2005 | } | 2026 | } | ||
2006 | } | 2027 | } | ||
2007 | else if (QT_STAT( QFile::encodeName(sPart), &buff ) == 0) { // should a very small ".part" be deleted? | 2028 | else if (partFile.exists()) { // should a very small ".part" be deleted? | ||
At this point we were doing another stat() in order to see how big the part file is *after* sftpGet. You need to call QFileInfo::refresh() first. dfaure: At this point we were doing another stat() in order to see how big the part file is *after*… | |||||
Better not do refresh on success, it slows things down for no purpose. else { partFile.refresh(); const int size = ...; if (partFile.exists() && partFile.size() < size) { partFile.remove(); } } dfaure: Better not do refresh on success, it slows things down for no purpose.
else {… | |||||
2008 | const int size = config()->readEntry("MinimumKeepSize", DEFAULT_MINIMUM_KEEP_SIZE); | 2029 | const int size = config()->readEntry("MinimumKeepSize", DEFAULT_MINIMUM_KEEP_SIZE); | ||
2009 | if (buff.st_size < size) | 2030 | if (partFile.size() < size) | ||
2010 | QFile::remove(sPart); | 2031 | QFile::remove(sPart); | ||
2011 | } | 2032 | } | ||
2012 | } | 2033 | } | ||
2013 | 2034 | | |||
2014 | const QString mtimeStr = metaData("modified"); | 2035 | const QString mtimeStr = metaData("modified"); | ||
2015 | if (!mtimeStr.isEmpty()) { | 2036 | if (!mtimeStr.isEmpty()) { | ||
2016 | QDateTime dt = QDateTime::fromString(mtimeStr, Qt::ISODate); | 2037 | QDateTime dt = QDateTime::fromString(mtimeStr, Qt::ISODate); | ||
2017 | if (dt.isValid()) { | 2038 | if (dt.isValid()) { | ||
2018 | struct utimbuf utbuf; | 2039 | QFile receivedFile(sCopyFile); | ||
2019 | utbuf.actime = buff.st_atime; // access time, unchanged | 2040 | if (receivedFile.exists()) { | ||
2020 | utbuf.modtime = dt.toTime_t(); // modification time | 2041 | if (!receivedFile.open(QIODevice::ReadWrite | QIODevice::Text)) { | ||
Why ReadWrite, if we know it doesn't exist? Does setFileTime() even need open() first? I wouldn't have thought so. dfaure: Why ReadWrite, if we know it doesn't exist?
Does setFileTime() even need open() first? I… | |||||
setFileTime() needs the file to be open. brute4s99: `setFileTime()` needs the file to be open.
Source : https://doc.qt.io/qt-5/qfiledevice. | |||||
2021 | utime(QFile::encodeName(sCopyFile), &utbuf); | 2042 | QString error_msg = receivedFile.errorString(); | ||
vonreth: well do something with the error string? | |||||
2043 | qCDebug(KIO_SFTP_LOG) << "Couldn't update modified time : " << error_msg; | ||||
no need for QStringLiteral here, sending a const char * to debug is perfectly fine; also, the qDebug must be categorized, just like all the other debug outputs pino: no need for QStringLiteral here, sending a `const char *` to debug is perfectly fine; also, the… | |||||
2044 | } | ||||
2045 | else { | ||||
dfaure: coding style: join with previous line
} else { | |||||
2046 | receivedFile.setFileTime(dt, QFileDevice::FileModificationTime); | ||||
dfaure: Again?? | |||||
Sorry I gave wrong advice. dfaure: Sorry I gave wrong advice.
The old code was preserving the atime just because the utime() API… | |||||
2047 | } | ||||
pino: what is this commented code for? | |||||
it uses buff.st_atime . Since I'm removing use of buff, I'm not sure how to handle this. For now I've commented out setting the file access time instruction for now. brute4s99: it uses buff.st_atime . Since I'm removing use of buff, I'm not sure how to handle this. For… | |||||
dfaure: Just use partFile.fileTime(QFileInfoFileAccessTime) | |||||
2048 | } | ||||
You can remove the FileTime() type conversion, just do setFileTime(dt, QFileDevice::FileModificationTime) dfaure: You can remove the FileTime() type conversion, just do setFileTime(dt, QFileDevice… | |||||
2022 | } | 2049 | } | ||
2023 | } | 2050 | } | ||
2024 | 2051 | | |||
2025 | return result; | 2052 | return result; | ||
2026 | } | 2053 | } | ||
2027 | 2054 | | |||
2028 | sftpProtocol::StatusCode sftpProtocol::sftpCopyPut(const QUrl& url, const QString& sCopyFile, int permissions, JobFlags flags, int& errorCode) | 2055 | sftpProtocol::StatusCode sftpProtocol::sftpCopyPut(const QUrl& url, const QString& sCopyFile, int permissions, JobFlags flags, int& errorCode) | ||
2029 | { | 2056 | { | ||
2030 | qCDebug(KIO_SFTP_LOG) << sCopyFile << "->" << url << ", permissions=" << permissions << ", flags" << flags; | 2057 | qCDebug(KIO_SFTP_LOG) << sCopyFile << "->" << url << ", permissions=" << permissions << ", flags" << flags; | ||
2031 | 2058 | | |||
2032 | // check if source is ok ... | 2059 | // check if source is ok ... | ||
2033 | QT_STATBUF buff; | 2060 | QFileInfo copyFile(sCopyFile); | ||
dfaure: = info.exists() | |||||
dfaure: Same here | |||||
2034 | bool bSrcExists = (QT_STAT(QFile::encodeName(sCopyFile), &buff) != -1); | 2061 | bool bSrcExists = copyFile.exists(); | ||
2035 | | ||||
2036 | if (bSrcExists) { | 2062 | if (bSrcExists) { | ||
2037 | if (S_ISDIR(buff.st_mode)) { | 2063 | if(copyFile.isDir()) { | ||
2038 | errorCode = ERR_IS_DIRECTORY; | 2064 | errorCode = ERR_IS_DIRECTORY; | ||
2039 | return sftpProtocol::ClientError; | 2065 | return sftpProtocol::ClientError; | ||
2040 | } | 2066 | } | ||
2041 | } else { | 2067 | } else { | ||
2042 | errorCode = ERR_DOES_NOT_EXIST; | 2068 | errorCode = ERR_DOES_NOT_EXIST; | ||
2043 | return sftpProtocol::ClientError; | 2069 | return sftpProtocol::ClientError; | ||
2044 | } | 2070 | } | ||
2045 | 2071 | | |||
2046 | const int fd = QT_OPEN(QFile::encodeName(sCopyFile), O_RDONLY); | 2072 | const int fd = QT_OPEN(QFile::encodeName(sCopyFile), O_RDONLY); | ||
2047 | if(fd == -1) | 2073 | if(fd == -1) | ||
2048 | { | 2074 | { | ||
2049 | errorCode = ERR_CANNOT_OPEN_FOR_READING; | 2075 | errorCode = ERR_CANNOT_OPEN_FOR_READING; | ||
2050 | return sftpProtocol::ClientError; | 2076 | return sftpProtocol::ClientError; | ||
2051 | } | 2077 | } | ||
2052 | 2078 | | |||
2053 | totalSize(buff.st_size); | 2079 | totalSize(copyFile.size()); | ||
2054 | 2080 | | |||
2055 | // delegate the real work (errorCode gets status) ... | 2081 | // delegate the real work (errorCode gets status) ... | ||
2056 | StatusCode ret = sftpPut(url, permissions, flags, errorCode, fd); | 2082 | StatusCode ret = sftpPut(url, permissions, flags, errorCode, fd); | ||
2057 | ::close(fd); | 2083 | ::close(fd); | ||
2058 | return ret; | 2084 | return ret; | ||
2059 | } | 2085 | } | ||
2060 | 2086 | | |||
2061 | 2087 | | |||
▲ Show 20 Lines • Show All 106 Lines • ▼ Show 20 Line(s) | 2151 | void sftpProtocol::listDir(const QUrl& url) { | |||
2168 | UDSEntry entry; | 2194 | UDSEntry entry; | ||
2169 | 2195 | | |||
2170 | qCDebug(KIO_SFTP_LOG) << "readdir: " << path << ", details: " << QString::number(details); | 2196 | qCDebug(KIO_SFTP_LOG) << "readdir: " << path << ", details: " << QString::number(details); | ||
2171 | 2197 | | |||
2172 | for (;;) { | 2198 | for (;;) { | ||
2173 | mode_t access; | 2199 | mode_t access; | ||
2174 | char *link; | 2200 | char *link; | ||
2175 | bool isBrokenLink = false; | 2201 | bool isBrokenLink = false; | ||
2176 | long long fileType = S_IFREG; | 2202 | long long fileType = QT_STAT_REG; | ||
2177 | long long size = 0LL; | 2203 | long long size = 0LL; | ||
2178 | 2204 | | |||
2179 | dirent = sftp_readdir(mSftp, dp); | 2205 | dirent = sftp_readdir(mSftp, dp); | ||
2180 | if (dirent == nullptr) { | 2206 | if (dirent == nullptr) { | ||
2181 | break; | 2207 | break; | ||
2182 | } | 2208 | } | ||
2183 | 2209 | | |||
2184 | entry.clear(); | 2210 | entry.clear(); | ||
Show All 19 Lines | 2229 | } else { | |||
2204 | sftp_attributes_free(dirent); | 2230 | sftp_attributes_free(dirent); | ||
2205 | dirent = sb; | 2231 | dirent = sb; | ||
2206 | } | 2232 | } | ||
2207 | } | 2233 | } | ||
2208 | } | 2234 | } | ||
2209 | 2235 | | |||
2210 | if (isBrokenLink) { | 2236 | if (isBrokenLink) { | ||
2211 | // It is a link pointing to nowhere | 2237 | // It is a link pointing to nowhere | ||
2212 | fileType = S_IFMT - 1; | 2238 | fileType = QT_STAT_MASK - 1; | ||
2239 | #ifdef Q_OS_WIN | ||||
2240 | access = static_cast<mode_t>(perms::owner_all | perms::group_all | perms::others_all); | ||||
vonreth: cast | |||||
2241 | #else | ||||
2213 | access = S_IRWXU | S_IRWXG | S_IRWXO; | 2242 | access = S_IRWXU | S_IRWXG | S_IRWXO; | ||
2243 | #endif | ||||
2214 | size = 0LL; | 2244 | size = 0LL; | ||
2215 | } else { | 2245 | } else { | ||
2216 | switch (dirent->type) { | 2246 | switch (dirent->type) { | ||
2217 | case SSH_FILEXFER_TYPE_REGULAR: | 2247 | case SSH_FILEXFER_TYPE_REGULAR: | ||
2218 | fileType = S_IFREG; | 2248 | fileType = QT_STAT_REG; | ||
2219 | break; | 2249 | break; | ||
2220 | case SSH_FILEXFER_TYPE_DIRECTORY: | 2250 | case SSH_FILEXFER_TYPE_DIRECTORY: | ||
2221 | fileType = S_IFDIR; | 2251 | fileType = QT_STAT_DIR; | ||
2222 | break; | 2252 | break; | ||
2223 | case SSH_FILEXFER_TYPE_SYMLINK: | 2253 | case SSH_FILEXFER_TYPE_SYMLINK: | ||
2224 | fileType = S_IFLNK; | 2254 | fileType = QT_STAT_LNK; | ||
2225 | break; | 2255 | break; | ||
vonreth: QFileInfo | |||||
I'm not sure what is intended to achieve here, the remote file _can_ be a symlink, and IIRC when we had sftp on dolphin in KDE4 those symlinks were shown as such. andriusr: I'm not sure what is intended to achieve here, the remote file _can_ be a symlink, and IIRC… | |||||
I think what they mean is that if you remove the ifdef it should just work. albertvaka: I think what they mean is that if you remove the ifdef it should just work. | |||||
brute4s99: ah, gotcha! | |||||
brute4s99: `S_IFLNK` is not defined on WIndows | |||||
vonreth: Does https://code.woboq.org/qt5/qtbase/mkspecs/common/posix/qplatformdefs.h.html#111 work?
| |||||
brute4s99: yes! thanks Hannah! 🎉 | |||||
You can keep QT_STAT_LNK on every platform and remove the ifdef. That's the point of the abstraction Qt provides. For consistency, I would also change the other (S_IFDIR, etc.) to their Qt counterparts. albertvaka: You can keep QT_STAT_LNK on every platform and remove the ifdef. That's the point of the… | |||||
This is marked as Done, but it's not Done. In fact it matches my own suggestion above, so I agree with Albert ;-) dfaure: This is marked as Done, but it's not Done. In fact it matches my own suggestion above, so I… | |||||
brute4s99: so sorry, it seems I updated a previous version of the diff. | |||||
2226 | case SSH_FILEXFER_TYPE_SPECIAL: | 2256 | case SSH_FILEXFER_TYPE_SPECIAL: | ||
2227 | case SSH_FILEXFER_TYPE_UNKNOWN: | 2257 | case SSH_FILEXFER_TYPE_UNKNOWN: | ||
2228 | break; | 2258 | break; | ||
2229 | } | 2259 | } | ||
2230 | 2260 | | |||
2231 | access = dirent->permissions & 07777; | 2261 | access = dirent->permissions & 07777; | ||
2232 | size = dirent->size; | 2262 | size = dirent->size; | ||
2233 | } | 2263 | } | ||
▲ Show 20 Lines • Show All 407 Lines • Show Last 20 Lines |
this builds without utime.h on my system (Arch Linux with latest Plasma, Qt and other packages), . Please inform if someone else has issues without this header.