Commit 70f2dc85 authored by Harald Sitter's avatar Harald Sitter 🌈
Browse files

smb: apply a whole bunch of static analyzer improvements

Summary:
among other things:

- {} initialize the posix structs
- =default d/ctors where appropriate
- synced argument names between header and cpp
- no c-style casting
- while(true) instead of while(1)
- initialize class members that could previously have gone uninitialized
- no single-line-multi-variable declaration

Test Plan: builds

Reviewers: ngraham

Reviewed By: ngraham

Subscribers: kde-frameworks-devel, kfm-devel

Tags: #dolphin, #frameworks

Differential Revision: https://phabricator.kde.org/D27646
parent 60c9ac5a
......@@ -46,7 +46,8 @@ KIO::UDSEntry DNSSDDiscovery::toEntry() const
QUrl u;
u.setScheme(QStringLiteral("smb"));
u.setHost(m_service->hostName());
if (m_service->port() > 0 && m_service->port() != 445 /* default smb */) {
const int defaultPort = 445;
if (m_service->port() > 0 && m_service->port() != defaultPort) {
u.setPort(m_service->port());
}
u.setPath("/"); // https://bugs.kde.org/show_bug.cgi?id=388922
......
......@@ -82,9 +82,7 @@ SMBSlave::SMBSlave(const QByteArray& pool, const QByteArray& app)
//===========================================================================
SMBSlave::~SMBSlave()
{
}
SMBSlave::~SMBSlave() = default;
void SMBSlave::virtual_hook(int id, void *data) {
switch(id) {
......
......@@ -128,7 +128,7 @@ private:
* else it crashes on exit next method after use cache_stat,
* looks like gcc (C/C++) failure
*/
struct stat st;
struct stat st{};
protected:
//---------------------------------------------
......@@ -248,7 +248,7 @@ public:
void reparseConfiguration() override;
// Functions overwritten in kio_smb_dir.cpp
void copy( const QUrl& src, const QUrl &dest, int permissions, KIO::JobFlags flags ) override;
void copy( const QUrl& src, const QUrl &dst, int permissions, KIO::JobFlags flags ) override;
void del( const QUrl& kurl, bool isfile) override;
void mkdir( const QUrl& kurl, int permissions ) override;
void rename( const QUrl& src, const QUrl& dest, KIO::JobFlags flags ) override;
......@@ -275,9 +275,9 @@ protected:
private:
SMBError errnumToKioError(const SMBUrl& url, const int errNum);
void smbCopy(const QUrl& src, const QUrl &dest, int permissions, KIO::JobFlags flags);
void smbCopyGet(const QUrl& src, const QUrl& dest, int permissions, KIO::JobFlags flags);
void smbCopyPut(const QUrl& src, const QUrl& dest, int permissions, KIO::JobFlags flags);
void smbCopy(const QUrl& src, const QUrl &dst, int permissions, KIO::JobFlags flags);
void smbCopyGet(const QUrl& ksrc, const QUrl& kdst, int permissions, KIO::JobFlags flags);
void smbCopyPut(const QUrl& ksrc, const QUrl& kdst, int permissions, KIO::JobFlags flags);
bool workaroundEEXIST(const int errNum) const;
void fileSystemFreeSpace(const QUrl &url);
......
......@@ -35,7 +35,7 @@
#include <kconfig.h>
#include <kconfiggroup.h>
#include <klocalizedstring.h>
#include <stdlib.h>
#include <cstdlib>
// call for libsmbclient
//==========================================================================
......@@ -48,9 +48,9 @@ void auth_smbc_get_data(SMBCCTX * context,
{
if (context != nullptr) {
#ifdef DEPRECATED_SMBC_INTERFACE
SMBSlave *theSlave = (SMBSlave*) smbc_getOptionUserData(context);
auto *theSlave = static_cast<SMBSlave*>(smbc_getOptionUserData(context));
#else
SMBSlave *theSlave = (SMBSlave*)smbc_option_get(context, "user_data");
auto *theSlave = static_cast<SMBSlave*>(smbc_option_get(context, "user_data"));
#endif
theSlave->auth_smbc_get_data(server, share,
workgroup,wgmaxlen,
......
......@@ -61,15 +61,13 @@ int SMBSlave::cache_stat(const SMBUrl &url, struct stat* st )
} else {
cacheStatErr = errno;
}
qCDebug(KIO_SMB_LOG) << "size " << (KIO::filesize_t)st->st_size;
qCDebug(KIO_SMB_LOG) << "size " << static_cast<KIO::filesize_t>(st->st_size);
return cacheStatErr;
}
//---------------------------------------------------------------------------
int SMBSlave::browse_stat_path(const SMBUrl& _url, UDSEntry& udsentry)
int SMBSlave::browse_stat_path(const SMBUrl &url, UDSEntry& udsentry)
{
SMBUrl url = _url;
int cacheStatErr = cache_stat(url, &st);
if(cacheStatErr == 0)
{
......@@ -292,7 +290,6 @@ SMBSlave::SMBError SMBSlave::errnumToKioError(const SMBUrl &url, const int errNu
#endif
case ECONNREFUSED:
return SMBError{ ERR_SLAVE_DEFINED, i18n("Could not connect to host for %1", url.toDisplayString()) };
break;
case ENOTDIR:
return SMBError{ ERR_CANNOT_ENTER_DIRECTORY, url.toDisplayString() };
case EFAULT:
......@@ -378,12 +375,11 @@ void SMBSlave::listDir( const QUrl& kurl )
m_current_url = kurl;
int dirfd;
struct smbc_dirent *dirp = nullptr;
UDSEntry udsentry;
bool dir_is_root = true;
dirfd = smbc_opendir( m_current_url.toSmbcUrl() );
int dirfd = smbc_opendir( m_current_url.toSmbcUrl() );
if (dirfd > 0){
errNum = 0;
} else {
......@@ -552,7 +548,7 @@ void SMBSlave::listDir( const QUrl& kurl )
const QList<Discoverer *> discoverers {&d, &w};
auto appendDiscovery = [&](Discovery::Ptr discovery) {
auto appendDiscovery = [&](const Discovery::Ptr &discovery) {
list.append(discovery->toEntry());
};
......@@ -644,7 +640,7 @@ void SMBSlave::fileSystemFreeSpace(const QUrl& url)
SMBUrl smbcUrl = url;
struct statvfs dirStat;
struct statvfs dirStat{};
memset(&dirStat, 0, sizeof(struct statvfs));
const int err = smbc_statvfs(smbcUrl.toSmbcUrl().data(), &dirStat);
if (err < 0) {
......
......@@ -176,8 +176,7 @@ void SMBSlave::smbCopy(const QUrl& ksrc, const QUrl& kdst, int permissions, KIO:
// Perform copy
while(1)
{
while (true) {
n = smbc_read(srcfd, buf, MAX_XFER_BUF_SIZE );
if(n > 0)
{
......@@ -360,7 +359,7 @@ void SMBSlave::smbCopyGet(const QUrl& ksrc, const QUrl& kdst, int permissions, K
char buf[MAX_XFER_BUF_SIZE];
bool isErr = false;
while (1) {
while (true) {
const ssize_t bytesRead = smbc_read(srcfd, buf, MAX_XFER_BUF_SIZE);
if (bytesRead <= 0) {
if (bytesRead < 0) {
......@@ -417,7 +416,7 @@ void SMBSlave::smbCopyGet(const QUrl& ksrc, const QUrl& kdst, int permissions, K
if (!mtimeStr.isEmpty()) {
QDateTime dt = QDateTime::fromString(mtimeStr, Qt::ISODate);
if (dt.isValid()) {
struct utimbuf utbuf;
struct utimbuf utbuf{};
utbuf.actime = QFileInfo(dstFile).lastRead().toSecsSinceEpoch(); // access time, unchanged
utbuf.modtime = dt.toSecsSinceEpoch(); // modification time
utime(QFile::encodeName(dstFile).constData(), &utbuf);
......@@ -541,7 +540,7 @@ void SMBSlave::smbCopyPut(const QUrl& ksrc, const QUrl& kdst, int permissions, K
// Perform the copy
char buf[MAX_XFER_BUF_SIZE];
while (1) {
while (true) {
const ssize_t bytesRead = srcFile.read(buf, MAX_XFER_BUF_SIZE);
if (bytesRead <= 0) {
if (bytesRead < 0) {
......@@ -601,7 +600,7 @@ void SMBSlave::smbCopyPut(const QUrl& ksrc, const QUrl& kdst, int permissions, K
if (!mtimeStr.isEmpty() ) {
QDateTime dt = QDateTime::fromString( mtimeStr, Qt::ISODate );
if ( dt.isValid() ) {
struct utimbuf utbuf;
struct utimbuf utbuf{};
utbuf.actime = st.st_atime; // access time, unchanged
utbuf.modtime = dt.toSecsSinceEpoch(); // modification time
smbc_utime( dstOrigUrl.toSmbcUrl(), &utbuf );
......
......@@ -91,8 +91,7 @@ void SMBSlave::get( const QUrl& kurl )
{
bool isFirstPacket = true;
// lasttime = starttime = time(NULL); // This seems to be unused..
while(1)
{
while (true) {
bytesread = smbc_read(filefd, buf, MAX_XFER_BUF_SIZE);
if(bytesread == 0)
{
......@@ -240,7 +239,7 @@ void SMBSlave::open( const QUrl& kurl, QIODevice::OpenMode mode)
}
position( 0 );
emit opened();
opened();
}
......@@ -418,8 +417,7 @@ void SMBSlave::put( const QUrl& kurl,
}
// Loop until we got 0 (end of data)
while(1)
{
while (true) {
qCDebug(KIO_SMB_LOG) << "request data ";
dataReq(); // Request for data
qCDebug(KIO_SMB_LOG) << "write " << m_current_url.toSmbcUrl();
......@@ -464,7 +462,7 @@ void SMBSlave::put( const QUrl& kurl,
QDateTime dt = QDateTime::fromString( mtimeStr, Qt::ISODate );
if ( dt.isValid() ) {
if (cache_stat( m_current_url, &st ) == 0) {
struct utimbuf utbuf;
struct utimbuf utbuf{};
utbuf.actime = st.st_atime; // access time, unchanged
utbuf.modtime = dt.toSecsSinceEpoch(); // modification time
smbc_utime( m_current_url.toSmbcUrl(), &utbuf );
......
......@@ -40,16 +40,8 @@
// SMBUrl Function Implementation
//===========================================================================
//-----------------------------------------------------------------------
SMBUrl::SMBUrl()
{
m_type = SMBURLTYPE_UNKNOWN;
}
//-----------------------------------------------------------------------
SMBUrl::SMBUrl(const QUrl& kurl)
SMBUrl::SMBUrl(const QUrl &kurl)
: QUrl(kurl)
//-----------------------------------------------------------------------
{
// We treat cifs as an alias but need to translate it to smb.
// https://bugs.kde.org/show_bug.cgi?id=327295
......@@ -62,14 +54,10 @@ SMBUrl::SMBUrl(const QUrl& kurl)
updateCache();
}
SMBUrl::SMBUrl(const SMBUrl& other)
: QUrl(other),
m_surl(other.m_surl),
m_type(other.m_type)
{
}
SMBUrl& SMBUrl::operator=(const SMBUrl&) = default;
SMBUrl::SMBUrl() = default;
SMBUrl::SMBUrl(const SMBUrl &other) = default;
SMBUrl::~SMBUrl() = default;
SMBUrl &SMBUrl::operator=(const SMBUrl &) = default;
//-----------------------------------------------------------------------
void SMBUrl::addPath(const QString &filedir)
......
......@@ -63,6 +63,7 @@ public:
SMBUrl();
SMBUrl(const SMBUrl&);
SMBUrl(const QUrl & kurl);
~SMBUrl();
SMBUrl& operator=(const SMBUrl&);
......@@ -121,7 +122,7 @@ private:
* Type of URL
* @see _SMBUrlType
*/
mutable SMBUrlType m_type;
mutable SMBUrlType m_type = SMBURLTYPE_UNKNOWN;
};
......
......@@ -40,11 +40,14 @@ void SMBSlave::special( const QByteArray & data)
case 1:
case 3:
{
QString remotePath, mountPoint, user;
QString remotePath;
QString mountPoint;
QString user;
stream >> remotePath >> mountPoint;
QStringList sl=remotePath.split('/');
QString share,host;
QString share;
QString host;
if (sl.count()>=2)
{
host=sl.at(0).mid(2);
......
Markdown is supported
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment