Skip to content

Use uint64_t for file size #680

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
TheOneRing wants to merge 2 commits into
base: main
Choose a base branch
from
Open

Use uint64_t for file size #680

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
dbcf708
Use uint64_t for file size
TheOneRing Nov 13, 2025
4cecaba
wip
TheOneRing Dec 11, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/gui/folderstatusmodel.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ namespace {
// item if no items are in progress.
SyncFileItem curItem = progress._lastCompletedItem;
qint64 curItemProgress = -1; // -1 means finished
qint64 biggerItemSize = 0;
uint64_t biggerItemSize = 0;
quint64 estimatedUpBw = 0;
quint64 estimatedDownBw = 0;
QStringList allFilenames;
Expand Down
7 changes: 7 additions & 0 deletions src/libsync/common/syncjournaldb.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1170,6 +1170,13 @@ static bool deleteBatch(SqlQuery &query, const QStringList &entries, const QStri
return true;
}

bool SyncJournalDb::UploadInfo::validate(uint64_t size, qint64 modtime, const QByteArray &checksum) const
{
Q_ASSERT(!checksum.isEmpty());
Q_ASSERT(!_valid || !_contentChecksum.isEmpty());
return _valid && _size == size && _modtime == modtime && _contentChecksum == checksum;
}

SyncJournalDb::DownloadInfo SyncJournalDb::getDownloadInfo(const QString &file)
{
QMutexLocker locker(&_mutex);
Expand Down
9 changes: 2 additions & 7 deletions src/libsync/common/syncjournaldb.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -106,18 +106,13 @@ class OPENCLOUD_SYNC_EXPORT SyncJournalDb : public QObject
{
bool _valid = false;

qint64 _size = 0;
uint64_t _size = 0;
qint64 _modtime = 0;
QByteArray _contentChecksum;
QUrl _url; // upload url (tus)
QString _path; // stored as utf16, used in local discovery

bool validate(qint64 size, qint64 modtime, const QByteArray &checksum) const
{
Q_ASSERT(!checksum.isEmpty());
Q_ASSERT(!_valid || !_contentChecksum.isEmpty());
return _valid && _size == size && _modtime == modtime && _contentChecksum == checksum;
}
bool validate(uint64_t size, qint64 modtime, const QByteArray &checksum) const;
};

DownloadInfo getDownloadInfo(const QString &file);
Expand Down
4 changes: 2 additions & 2 deletions src/libsync/common/syncjournalfilerecord.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ class OCC::SyncJournalFileRecordData : public QSharedData
ItemType _type = ItemTypeUnsupported;
QString _etag;
QByteArray _fileId;
qint64 _fileSize = 0;
uint64_t _fileSize = 0;
RemotePermissions _remotePerm;
bool _serverHasIgnoredFiles = false;
bool _hasDirtyPlaceholder = false;
Expand Down Expand Up @@ -195,7 +195,7 @@ time_t SyncJournalFileRecord::modtime() const
return d->_modtime.value_or(0);
}

int64_t SyncJournalFileRecord::size() const
uint64_t SyncJournalFileRecord::size() const
{
return d->_fileSize;
}
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/common/syncjournalfilerecord.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ class OPENCLOUD_SYNC_EXPORT SyncJournalFileRecord
RemotePermissions remotePerm() const;
QByteArray checksumHeader() const;
time_t modtime() const;
int64_t size() const;
uint64_t size() const;
uint64_t inode() const;
ItemType type() const;

Expand Down
8 changes: 3 additions & 5 deletions src/libsync/common/utility.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -65,18 +65,16 @@ namespace OCC {

Q_LOGGING_CATEGORY(lcUtility, "sync.utility", QtMsgType::QtInfoMsg)

QString Utility::octetsToString(qint64 octets)
QString Utility::octetsToString(uint64_t octets)
{
OC_ASSERT(octets >= 0)

using namespace FileSystem::SizeLiterals;

// We do what macOS 10.8 and above do: 0 fraction digits for bytes and KB; 1 fraction digits for MB; 2 for GB and above.
// See also https://developer.apple.com/documentation/foundation/nsbytecountformatter/1417887-adaptive
int precision = 0;
if (quint64(octets) >= 1_GiB) {
if (octets >= 1_GiB) {
precision = 2;
} else if (quint64(octets) >= 1_MiB) {
} else if (octets >= 1_MiB) {
precision = 1;
}

Expand Down
3 changes: 1 addition & 2 deletions src/libsync/common/utility.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,6 @@
#include <QMap>
#include <QMetaEnum>
#include <QString>
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The include #include <QUrl> was removed, but the header still uses QUrl as a parameter type in function declarations on lines 204 and 207. While QUrlQuery includes QUrl transitively in some Qt versions, relying on transitive includes is fragile and can break with different Qt versions or build configurations. The #include <QUrl> should be kept.

Suggested change
#include <QString>
#include <QString>
#include <QUrl>

Copilot uses AI. Check for mistakes.
#include <QUrl>
#include <QUrlQuery>

#include <functional>
Expand All @@ -50,7 +49,7 @@ OPENCLOUD_SYNC_EXPORT Q_DECLARE_LOGGING_CATEGORY(lcUtility)
namespace Utility
{
OPENCLOUD_SYNC_EXPORT void setupFavLink(const QString &folder);
OPENCLOUD_SYNC_EXPORT QString octetsToString(qint64 octets);
OPENCLOUD_SYNC_EXPORT QString octetsToString(uint64_t octets);
OPENCLOUD_SYNC_EXPORT QByteArray userAgentString();

/**
Expand Down
5 changes: 3 additions & 2 deletions src/libsync/discoveryinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@ class OCC::LocalInfoData : public QSharedData
return;
}
_inode = sb.st_ino;
Q_ASSERT(sb.st_size >= 0);
_size = sb.st_size;
_modtime = sb.st_mtime;
#ifdef Q_OS_MAC
Expand All @@ -65,7 +66,7 @@ class OCC::LocalInfoData : public QSharedData
QString _name;
ItemType _type = ItemTypeUnsupported;
time_t _modtime = 0;
int64_t _size = 0;
uint64_t _size = 0;
uint64_t _inode = 0;
bool _isHidden = false;
};
Expand Down Expand Up @@ -129,7 +130,7 @@ time_t LocalInfo::modtime() const
return d->_modtime;
}

int64_t LocalInfo::size() const
uint64_t LocalInfo::size() const
{
return d->_size;
}
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/discoveryinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ class OPENCLOUD_SYNC_EXPORT LocalInfo
/** FileName of the entry (this does not contain any directory or path, just the plain name */
QString name() const;
time_t modtime() const;
int64_t size() const;
uint64_t size() const;
uint64_t inode() const;
ItemType type() const;
bool isDirectory() const;
Expand Down
7 changes: 3 additions & 4 deletions src/libsync/discoveryremoteinfo.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,8 +31,7 @@ class OCC::RemoteInfoData : public QSharedData
_size = 0;
} else {
if (auto it = Utility::optionalFind(map, "getcontentlength"_L1)) {
// See #4573, sometimes negative size values are returned
_size = std::max<int64_t>(0, it->value().toLongLong());
_size = it->value().toULongLong();
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The removal of the check std::max<int64_t>(0, it->value().toLongLong()) that protected against negative size values from the server (see comment "See #4573, sometimes negative size values are returned") may introduce issues. While toULongLong() will convert negative values to large positive numbers due to two's complement representation, this could result in incorrect behavior. Consider adding validation to ensure the parsed value is sensible, or at minimum document why this protection is no longer needed.

Copilot uses AI. Check for mistakes.
} else {
errors.append(u"size"_s);
}
Expand Down Expand Up @@ -74,7 +73,7 @@ class OCC::RemoteInfoData : public QSharedData
QByteArray _checksumHeader;
RemotePermissions _remotePerm;
time_t _modtime = 0;
int64_t _size = 0;
uint64_t _size = 0;
bool _isDirectory = false;

QString _error;
Expand Down Expand Up @@ -134,7 +133,7 @@ time_t RemoteInfo::modtime() const
return d->_modtime;
}

int64_t RemoteInfo::size() const
uint64_t RemoteInfo::size() const
{
return d->_size;
}
Expand Down
2 changes: 1 addition & 1 deletion src/libsync/discoveryremoteinfo.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ class OPENCLOUD_SYNC_EXPORT RemoteInfo
RemotePermissions remotePerm() const;
time_t modtime() const;

int64_t size() const;
uint64_t size() const;

QString error() const;

Expand Down
2 changes: 1 addition & 1 deletion src/libsync/filesystem.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,7 @@ namespace FileSystem {
static OPENCLOUD_SYNC_EXPORT FileChangedInfo fromSyncFileItemPrevious(const SyncFileItem *const item);
static OPENCLOUD_SYNC_EXPORT FileChangedInfo fromSyncJournalFileRecord(const SyncJournalFileRecord &record);

qint64 size = {};
uint64_t size = {};
std::optional<time_t> mtime = {};
std::optional<quint64> inode = {};
CSyncEnums::ItemType type = CSyncEnums::ItemTypeUnsupported;
Expand Down
32 changes: 26 additions & 6 deletions src/libsync/networkjobs/getfilejob.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,8 +17,6 @@ GETFileJob::GETFileJob(AccountPtr account, const QUrl &url, const QString &path,
, _device(device)
, _headers(headers)
, _expectedEtagForResume(expectedEtagForResume)
, _expectedContentLength(-1)
, _contentLength(-1)
, _resumeStart(resumeStart)
{
connect(this, &GETFileJob::networkError, this, [this] {
Expand Down Expand Up @@ -83,6 +81,26 @@ void GETFileJob::newReplyHook(QNetworkReply *reply)
connect(reply, &QNetworkReply::downloadProgress, this, &GETFileJob::downloadProgress);
}

uint64_t GETFileJob::resumeStart() const
{
return _resumeStart;
}

std::optional<uint64_t> GETFileJob::contentLength() const
{
return _contentLength;
}

std::optional<uint64_t> GETFileJob::expectedContentLength() const
{
return _expectedContentLength;
}

void GETFileJob::setExpectedContentLength(uint64_t size)
{
_expectedContentLength = size;
}

void GETFileJob::slotMetaDataChanged()
{
// For some reason setting the read buffer in GETFileJob::start doesn't seem to go
Expand Down Expand Up @@ -132,22 +150,24 @@ void GETFileJob::slotMetaDataChanged()
}

bool ok;
_contentLength = reply()->header(QNetworkRequest::ContentLengthHeader).toLongLong(&ok);
if (ok && _expectedContentLength != -1 && _contentLength != _expectedContentLength) {
_contentLength = reply()->header(QNetworkRequest::ContentLengthHeader).toULongLong(&ok);
if (!ok) {
_contentLength.reset();
} else if (_expectedContentLength.has_value() && _contentLength != _expectedContentLength) {
qCWarning(lcGetJob) << u"We received a different content length than expected!" << _expectedContentLength << u"vs" << _contentLength;
_errorString = tr("We received an unexpected download Content-Length.");
_errorStatus = SyncFileItem::NormalError;
abort();
return;
}

qint64 start = 0;
uint64_t start = 0;
const QString ranges = QString::fromUtf8(reply()->rawHeader("Content-Range"));
if (!ranges.isEmpty()) {
static QRegularExpression rx(QStringLiteral("bytes (\\d+)-"));
const auto match = rx.match(ranges);
if (match.hasMatch()) {
start = match.captured(1).toLongLong();
start = match.captured(1).toULongLong();
}
}
if (start != _resumeStart) {
Expand Down
14 changes: 7 additions & 7 deletions src/libsync/networkjobs/getfilejob.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,9 +21,9 @@ class OPENCLOUD_SYNC_EXPORT GETFileJob : public AbstractNetworkJob
QIODevice *_device;
QMap<QByteArray, QByteArray> _headers;
QString _expectedEtagForResume;
qint64 _expectedContentLength;
qint64 _contentLength;
qint64 _resumeStart;
std::optional<uint64_t> _expectedContentLength;
std::optional<uint64_t> _contentLength;
uint64_t _resumeStart;

public:
// DOES NOT take ownership of the device.
Expand All @@ -39,11 +39,11 @@ class OPENCLOUD_SYNC_EXPORT GETFileJob : public AbstractNetworkJob

void newReplyHook(QNetworkReply *reply) override;

qint64 resumeStart() { return _resumeStart; }
uint64_t resumeStart() const;

qint64 contentLength() const { return _contentLength; }
qint64 expectedContentLength() const { return _expectedContentLength; }
void setExpectedContentLength(qint64 size) { _expectedContentLength = size; }
std::optional<uint64_t> contentLength() const;
std::optional<uint64_t> expectedContentLength() const;
void setExpectedContentLength(uint64_t size);

void setChoked(bool c);
void setBandwidthLimited(bool b);
Expand Down
7 changes: 4 additions & 3 deletions src/libsync/owncloudpropagator.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
#include "common/utility.h"
#include "discoveryphase.h"
#include "filesystem.h"
#include "libsync/common/filesystembase.h"
#include "propagatedownload.h"
#include "propagateremotedelete.h"
#include "propagateremotemkdir.h"
Expand All @@ -42,6 +43,7 @@
#include <qmath.h>

using namespace std::chrono_literals;
using namespace OCC::FileSystem::SizeLiterals;

namespace OCC {

Expand Down Expand Up @@ -355,10 +357,9 @@ PropagateItemJob *OwncloudPropagator::createJob(const SyncFileItemPtr &item)
Q_UNREACHABLE();
}

qint64 OwncloudPropagator::smallFileSize()
uint64_t OwncloudPropagator::smallFileSize()
{
const qint64 smallFileSize = 100 * 1024; //default to 1 MB. Not dynamic right now.
return smallFileSize;
return 1_MB; // default to 1 MB. Not dynamic right now.
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The comment states "default to 1 MB" but the original code was 100 * 1024 which equals 102,400 bytes (100 KiB), not 1 MB. Using 1_MB (decimal megabyte = 1,000,000 bytes) changes the behavior significantly. This should likely be 100_KiB to maintain the original behavior, or the value should be intentionally changed to 1_MiB (1,048,576 bytes) if that was the intent.

Suggested change
return 1_MB; // default to 1 MB. Not dynamic right now.
return 100_KiB; // default to 100 KiB. Not dynamic right now.

Copilot uses AI. Check for mistakes.
}

/**
Expand Down
10 changes: 2 additions & 8 deletions src/libsync/owncloudpropagator.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,12 +16,6 @@

#include <QHash>
#include <QObject>
#include <QMap>
#include <QElapsedTimer>
#include <QTimer>
#include <QPointer>
#include <QIODevice>
#include <QMutex>

#include "accountfwd.h"
#include "bandwidthmanager.h"
Expand Down Expand Up @@ -431,7 +425,7 @@ class OPENCLOUD_SYNC_EXPORT OwncloudPropagator : public QObject
*
* This allows skipping of uploads that have a very high likelihood of failure.
*/
QHash<QString, qint64> _folderQuota;
QHash<QString, uint64_t> _folderQuota;

/* the maximum number of jobs using bandwidth (uploads or downloads, in parallel) */
int maximumActiveTransferJob();
Expand All @@ -443,7 +437,7 @@ class OPENCLOUD_SYNC_EXPORT OwncloudPropagator : public QObject
* chunk-upload duration set.
*/
qint64 _chunkSize;
qint64 smallFileSize();
uint64_t smallFileSize();

/* The maximum number of active jobs in parallel */
int hardMaximumActiveJob();
Expand Down
4 changes: 2 additions & 2 deletions src/libsync/propagatedownload.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -387,7 +387,7 @@ void PropagateDownloadFile::startFullDownload()
qint64 PropagateDownloadFile::committedDiskSpace() const
Copy link

Copilot AI Dec 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The function return type is qint64 but qBound<uint64_t> is used. This creates a type mismatch where an unsigned value is being implicitly converted to a signed return type. If the bounded result exceeds INT64_MAX, it will result in undefined behavior or incorrect negative values. The return type should be changed to uint64_t to match the type parameter used in qBound.

Suggested change
qint64 PropagateDownloadFile::committedDiskSpace() const
uint64_t PropagateDownloadFile::committedDiskSpace() const

Copilot uses AI. Check for mistakes.
{
if (state() == Running) {
return qBound(0LL, _item->_size - _resumeStart - _downloadProgress, _item->_size);
return qBound<uint64_t>(0, _item->_size - _resumeStart - _downloadProgress, _item->_size);
}
return 0;
}
Expand Down Expand Up @@ -481,7 +481,7 @@ void PropagateDownloadFile::slotGetFinished()
*/
const QByteArray sizeHeader("Content-Length");
bool hasSizeHeader = true;
qint64 bodySize = job->reply()->rawHeader(sizeHeader).toLongLong(&hasSizeHeader);
uint64_t bodySize = job->reply()->rawHeader(sizeHeader).toULongLong(&hasSizeHeader);

// Qt removes the content-length header for transparently decompressed HTTP1 replies
// but not for HTTP2 or SPDY replies. For these it remains and contains the size
Expand Down
5 changes: 2 additions & 3 deletions src/libsync/propagatedownload.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
#include "networkjobs.h"
#include "owncloudpropagator.h"

#include <QBuffer>
#include <QFile>

namespace OCC {
Expand Down Expand Up @@ -119,8 +118,8 @@ private Q_SLOTS:
private:
void deleteExistingFolder();

qint64 _resumeStart;
qint64 _downloadProgress;
uint64_t _resumeStart;
uint64_t _downloadProgress;
QPointer<GETFileJob> _job;
QFile _tmpFile;
bool _deleteExisting;
Expand Down
4 changes: 2 additions & 2 deletions src/libsync/propagateupload.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,7 @@ void PropagateUploadFileCommon::start()
}

// Check if we believe that the upload will fail due to remote quota limits
const qint64 quotaGuess = propagator()->_folderQuota.value(QFileInfo(_item->localName()).path(), std::numeric_limits<qint64>::max());
const uint64_t quotaGuess = propagator()->_folderQuota.value(QFileInfo(_item->localName()).path(), std::numeric_limits<uint64_t>::max());
if (_item->_size > quotaGuess) {
// Necessary for blacklisting logic
_item->_httpErrorCode = 507;
Expand Down Expand Up @@ -449,7 +449,7 @@ void PropagateUploadFileCommon::commonErrorHandling(AbstractNetworkJob *job)
const auto path = QFileInfo(_item->localName()).path();
auto quotaIt = propagator()->_folderQuota.find(path);
if (quotaIt != propagator()->_folderQuota.end()) {
quotaIt.value() = qMin(quotaIt.value(), _item->_size - 1);
quotaIt.value() = qMin<uint64_t>(quotaIt.value(), _item->_size - 1);
} else {
propagator()->_folderQuota[path] = _item->_size - 1;
}
Expand Down
Loading