Skip to content

Commit

Permalink
Speed value overflow (#314)
Browse files Browse the repository at this point in the history
- fixed speed value overflows if the download speed is greater than 2 GB/s;
- the `Status` xml-rpc response no has the following properties: `DownloadRateLo`, `DownloadRateHi`, `AverageDownloadRateLo`, `AverageDownloadRateHi`. `DownloadRate` and `AverageDownloadRate` are deprecated now.
  • Loading branch information
dnzbk committed Jul 22, 2024
1 parent a7ac9a9 commit 6d6d973
Show file tree
Hide file tree
Showing 13 changed files with 224 additions and 49 deletions.
5 changes: 3 additions & 2 deletions daemon/frontend/ColoredFrontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*
* Copyright (C) 2004 Sven Henkel <sidddy@users.sourceforge.net>
* Copyright (C) 2007-2016 Andrey Prygunkov <hugbug@users.sourceforge.net>
* Copyright (C) 2024 Denis <denis@nzbget.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand All @@ -15,7 +16,7 @@
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/


Expand Down Expand Up @@ -51,7 +52,7 @@ void ColoredFrontend::BeforePrint()
void ColoredFrontend::PrintStatus()
{
BString<100> timeString;
int currentDownloadSpeed = m_standBy ? 0 : m_currentDownloadSpeed;
int64 currentDownloadSpeed = m_standBy ? 0 : m_currentDownloadSpeed;

if (currentDownloadSpeed > 0 && !m_pauseDownload)
{
Expand Down
5 changes: 3 additions & 2 deletions daemon/frontend/Frontend.h
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*
* Copyright (C) 2004 Sven Henkel <sidddy@users.sourceforge.net>
* Copyright (C) 2007-2016 Andrey Prygunkov <hugbug@users.sourceforge.net>
* Copyright (C) 2024 Denis <denis@nzbget.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand All @@ -15,7 +16,7 @@
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/


Expand All @@ -42,7 +43,7 @@ class Frontend : public Thread
int m_updateInterval;

// summary
int m_currentDownloadSpeed = 0;
int64 m_currentDownloadSpeed = 0;
int64 m_remainingSize = 0;
bool m_pauseDownload = false;
int m_downloadLimit = 0;
Expand Down
13 changes: 7 additions & 6 deletions daemon/frontend/NCursesFrontend.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
*
* Copyright (C) 2004 Sven Henkel <sidddy@users.sourceforge.net>
* Copyright (C) 2007-2019 Andrey Prygunkov <hugbug@users.sourceforge.net>
* Copyright (C) 2024 Denis <denis@nzbget.com>
*
* This program is free software; you can redistribute it and/or modify
* it under the terms of the GNU General Public License as published by
Expand All @@ -15,7 +16,7 @@
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/


Expand Down Expand Up @@ -539,10 +540,10 @@ void NCursesFrontend::PrintStatus()

BString<100> timeString;

int currentDownloadSpeed = m_standBy ? 0 : m_currentDownloadSpeed;
int64 currentDownloadSpeed = m_standBy ? 0 : m_currentDownloadSpeed;
if (currentDownloadSpeed > 0 && !m_pauseDownload)
{
int64 remain_sec = (int64)(m_remainingSize / currentDownloadSpeed);
int64 remain_sec = m_remainingSize / currentDownloadSpeed;
int h = (int)(remain_sec / 3600);
int m = (int)((remain_sec % 3600) / 60);
int s = (int)(remain_sec % 60);
Expand All @@ -561,7 +562,7 @@ void NCursesFrontend::PrintStatus()
postStatus.Format(", %i post-job%s", m_postJobCount, m_postJobCount > 1 ? "s" : "");
}

int averageSpeed = (int)(m_dnTimeSec > 0 ? m_allBytes / m_dnTimeSec : 0);
int64 averageSpeed = m_dnTimeSec > 0 ? (m_allBytes / m_dnTimeSec) : 0;

BString<1024> status(" %d threads, %s, %s remaining%s%s%s%s, Avg. %s",
m_threadCount, *Util::FormatSpeed(currentDownloadSpeed),
Expand Down Expand Up @@ -920,15 +921,15 @@ void NCursesFrontend::PrintGroupname(NzbInfo* nzbInfo, int row, bool selected, b
CString total = Util::FormatSize(nzbInfo->GetSize());

BString<100> time;
int currentDownloadSpeed = m_standBy ? 0 : m_currentDownloadSpeed;
int64 currentDownloadSpeed = m_standBy ? 0 : m_currentDownloadSpeed;
if (nzbInfo->GetPausedSize() > 0 && unpausedRemainingSize == 0)
{
time = "[paused]";
remaining = Util::FormatSize(nzbInfo->GetRemainingSize());
}
else if (currentDownloadSpeed > 0 && !m_pauseDownload)
{
int64 remain_sec = (int64)(unpausedRemainingSize / currentDownloadSpeed);
int64 remain_sec = unpausedRemainingSize / currentDownloadSpeed;
int h = (int)(remain_sec / 3600);
int m = (int)((remain_sec % 3600) / 60);
int s = (int)(remain_sec % 60);
Expand Down
2 changes: 2 additions & 0 deletions daemon/main/nzbget.h
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,8 @@ compiled */
#include <chrono>
#include <optional>
#include <variant>
#include <limits>
#include <type_traits>

#include <libxml/parser.h>
#include <libxml/xmlreader.h>
Expand Down
10 changes: 5 additions & 5 deletions daemon/nntp/StatMeter.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -278,27 +278,27 @@ void StatMeter::CalcTotalStat(int* upTimeSec, int* dnTimeSec, int64* allBytes, b
}

// Average speed in last 30 seconds
int StatMeter::CalcCurrentDownloadSpeed()
int64 StatMeter::CalcCurrentDownloadSpeed()
{
if (m_standBy)
{
return 0;
}

int timeDiff = (int)Util::CurrentTime() - m_speedStartTime * SPEEDMETER_SLOTSIZE;
int64 timeDiff = static_cast<int64>(Util::CurrentTime()) - m_speedStartTime * SPEEDMETER_SLOTSIZE;
if (timeDiff == 0)
{
return 0;
}

return (int)(m_speedTotalBytes / timeDiff);
return m_speedTotalBytes / timeDiff;
}

// Amount of data downloaded in current second
int StatMeter::CalcMomentaryDownloadSpeed()
int64 StatMeter::CalcMomentaryDownloadSpeed()
{
time_t curTime = Util::CurrentTime();
int speed = curTime == m_curSecTime ? m_curSecBytes.load() : 0;
int64 speed = curTime == m_curSecTime ? m_curSecBytes.load() : 0;
return speed;
}

Expand Down
4 changes: 2 additions & 2 deletions daemon/nntp/StatMeter.h
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,8 @@ class StatMeter : public Debuggable
public:
StatMeter();
void Init();
int CalcCurrentDownloadSpeed();
int CalcMomentaryDownloadSpeed();
int64 CalcCurrentDownloadSpeed();
int64 CalcMomentaryDownloadSpeed();
void AddSpeedReading(int bytes);
void AddServerData(int bytes, int serverId);
void CalcTotalStat(int* upTimeSec, int* dnTimeSec, int64* allBytes, bool* standBy);
Expand Down
2 changes: 1 addition & 1 deletion daemon/remote/BinRpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -671,7 +671,7 @@ void ListBinCommand::Execute()
}

uint32 sizeHi, sizeLo;
ListResponse.m_downloadRate = htonl(g_StatMeter->CalcCurrentDownloadSpeed());
ListResponse.m_downloadRate = htonl(Util::SafeIntCast<int64, uint32>(g_StatMeter->CalcCurrentDownloadSpeed()));
Util::SplitInt64(remainingSize, &sizeHi, &sizeLo);
ListResponse.m_remainingSizeHi = htonl(sizeHi);
ListResponse.m_remainingSizeLo = htonl(sizeLo);
Expand Down
42 changes: 31 additions & 11 deletions daemon/remote/XmlRpc.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
* GNU General Public License for more details.
*
* You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>.
* along with this program. If not, see <https://www.gnu.org/licenses/>.
*/


Expand Down Expand Up @@ -1256,8 +1256,12 @@ void StatusXmlCommand::Execute()
"<member><name>ArticleCacheLo</name><value><i4>%u</i4></value></member>\n"
"<member><name>ArticleCacheHi</name><value><i4>%u</i4></value></member>\n"
"<member><name>ArticleCacheMB</name><value><i4>%i</i4></value></member>\n"
"<member><name>DownloadRate</name><value><i4>%i</i4></value></member>\n"
"<member><name>AverageDownloadRate</name><value><i4>%i</i4></value></member>\n"
"<member><name>DownloadRate</name><value><i4>%i</i4></value></member>\n" // deprecated
"<member><name>DownloadRateLo</name><value><i4>%i</i4></value></member>\n"
"<member><name>DownloadRateHi</name><value><i4>%i</i4></value></member>\n"
"<member><name>AverageDownloadRate</name><value><i4>%i</i4></value></member>\n" // deprecated
"<member><name>AverageDownloadRateLo</name><value><i4>%i</i4></value></member>\n"
"<member><name>AverageDownloadRateHi</name><value><i4>%i</i4></value></member>\n"
"<member><name>DownloadLimit</name><value><i4>%i</i4></value></member>\n"
"<member><name>ThreadCount</name><value><i4>%u</i4></value></member>\n"
"<member><name>ParJobCount</name><value><i4>%i</i4></value></member>\n" // deprecated (renamed to PostJobCount)
Expand Down Expand Up @@ -1305,18 +1309,22 @@ void StatusXmlCommand::Execute()
"\"ArticleCacheLo\" : %u,\n"
"\"ArticleCacheHi\" : %u,\n"
"\"ArticleCacheMB\" : %i,\n"
"\"DownloadRate\" : %i,\n"
"\"AverageDownloadRate\" : %i,\n"
"\"DownloadRate\" : %i,\n" // deprecated
"\"DownloadRateLo\" : %u,\n"
"\"DownloadRateHi\" : %u,\n"
"\"AverageDownloadRate\" : %i,\n" // deprecated
"\"AverageDownloadRateLo\" : %u,\n"
"\"AverageDownloadRateHi\" : %u,\n"
"\"DownloadLimit\" : %i,\n"
"\"ThreadCount\" : %u,\n"
"\"ParJobCount\" : %i,\n" // deprecated (renamed to PostJobCount)
"\"ParJobCount\" : %i,\n" // deprecated (renamed to PostJobCount)
"\"PostJobCount\" : %i,\n"
"\"UrlCount\" : %i,\n"
"\"UpTimeSec\" : %i,\n"
"\"DownloadTimeSec\" : %i,\n"
"\"ServerPaused\" : %s,\n" // deprecated (renamed to DownloadPaused)
"\"ServerPaused\" : %s,\n" // deprecated (renamed to DownloadPaused)
"\"DownloadPaused\" : %s,\n"
"\"Download2Paused\" : %s,\n" // deprecated (same as DownloadPaused)
"\"Download2Paused\" : %s,\n" // deprecated (same as DownloadPaused)
"\"ServerStandBy\" : %s,\n"
"\"PostPaused\" : %s,\n"
"\"ScanPaused\" : %s,\n"
Expand Down Expand Up @@ -1372,7 +1380,10 @@ void StatusXmlCommand::Execute()
Util::SplitInt64(articleCache, &articleCacheHi, &articleCacheLo);
int articleCacheMBytes = (int)(articleCache / 1024 / 1024);

int downloadRate = (int)(g_StatMeter->CalcCurrentDownloadSpeed());
int64 downloadRate = g_StatMeter->CalcCurrentDownloadSpeed();
uint32 downloadRateHi, downloadRateLo;
Util::SplitInt64(downloadRate, &downloadRateHi, &downloadRateLo);

int downloadLimit = (int)(g_WorkState->GetSpeedLimit());
bool downloadPaused = g_WorkState->GetPauseDownload();
bool postPaused = g_WorkState->GetPausePostProcess();
Expand All @@ -1387,7 +1398,10 @@ void StatusXmlCommand::Execute()
g_StatMeter->CalcTotalStat(&upTimeSec, &downloadTimeSec, &allBytes, &serverStandBy);
int downloadedMBytes = (int)(allBytes / 1024 / 1024);
Util::SplitInt64(allBytes, &downloadedSizeHi, &downloadedSizeLo);
int averageDownloadRate = (int)(downloadTimeSec > 0 ? allBytes / downloadTimeSec : 0);

int64 averageDownloadRate = downloadTimeSec > 0 ? allBytes / downloadTimeSec : 0;
uint32 averageDownloadRateHi, averageDownloadRateLo;
Util::SplitInt64(averageDownloadRate, &averageDownloadRateHi, &averageDownloadRateLo);

int64 monthBytes, dayBytes;
g_StatMeter->CalcQuotaUsage(monthBytes, dayBytes);
Expand All @@ -1414,7 +1428,13 @@ void StatusXmlCommand::Execute()
forcedSizeHi, forcedMBytes, downloadedSizeLo, downloadedSizeHi, downloadedMBytes,
monthSizeLo, monthSizeHi, monthMBytes, daySizeLo, daySizeHi, dayMBytes,
articleCacheLo, articleCacheHi, articleCacheMBytes,
downloadRate, averageDownloadRate, downloadLimit, threadCount,
Util::SafeIntCast<int64, int32>(downloadRate),
downloadRateLo,
downloadRateHi,
Util::SafeIntCast<int64, int32>(averageDownloadRate),
averageDownloadRateLo,
averageDownloadRateHi,
downloadLimit, threadCount,
postJobCount, postJobCount, urlCount, upTimeSec, downloadTimeSec,
BoolToStr(downloadPaused), BoolToStr(downloadPaused), BoolToStr(downloadPaused),
BoolToStr(serverStandBy), BoolToStr(postPaused), BoolToStr(scanPaused), BoolToStr(quotaReached),
Expand Down
25 changes: 21 additions & 4 deletions daemon/util/Util.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -336,21 +336,38 @@ CString Util::FormatSize(int64 fileSize)
return result;
}

CString Util::FormatSpeed(int bytesPerSecond)
CString Util::FormatSpeed(int64 bytesPerSecond)
{
CString result;

if (bytesPerSecond >= 100 * 1024 * 1024)
if (bytesPerSecond <= 0)
{
return result;
}

if (bytesPerSecond >= 100ll * 1024 * 1024 * 1024)
{
result.Format("%i GB/s", bytesPerSecond / 1024 / 1024 / 1024);
}
else if (bytesPerSecond >= 10ll * 1024 * 1024 * 1024)
{
result.Format("%0.1f GB/s", static_cast<double>(bytesPerSecond) / 1024 / 1024 / 1024);
}
else if (bytesPerSecond >= 1024 * 1024 * 1024)
{
result.Format("%0.2f GB/s", static_cast<double>(bytesPerSecond) / 1024 / 1024 / 1024);
}
else if (bytesPerSecond >= 100 * 1024 * 1024)
{
result.Format("%i MB/s", bytesPerSecond / 1024 / 1024);
}
else if (bytesPerSecond >= 10 * 1024 * 1024)
{
result.Format("%0.1f MB/s", (float)bytesPerSecond / 1024.0 / 1024.0);
result.Format("%0.1f MB/s", static_cast<double>(bytesPerSecond) / 1024 / 1024);
}
else if (bytesPerSecond >= 1024 * 1000)
{
result.Format("%0.2f MB/s", (float)bytesPerSecond / 1024.0 / 1024.0);
result.Format("%0.2f MB/s", static_cast<double>(bytesPerSecond) / 1024 / 1024);
}
else
{
Expand Down
20 changes: 19 additions & 1 deletion daemon/util/Util.h
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
#define UTIL_H

#include <optional>
#include <type_traits>
#include "NString.h"

#ifdef WIN32
Expand Down Expand Up @@ -59,6 +60,23 @@ class Util
static bool EndsWith(const char* str, const char* suffix, bool caseSensitive);
static bool AlphaNum(const char* str);

template <typename From, typename To,
typename std::enable_if_t<std::is_integral_v<From> && std::is_integral_v<To>, bool> = true>
static To SafeIntCast(From num)
{
if (num > std::numeric_limits<To>::max())
{
return 0;
}

if (num < std::numeric_limits<To>::min())
{
return 0;
}

return static_cast<To>(num);
}

/* replace all occurences of szFrom to szTo in string szStr with a limitation that szTo must be shorter than szFrom */
static char* ReduceStr(char* str, const char* from, const char* to);

Expand All @@ -83,7 +101,7 @@ class Util
static void FormatTime(time_t timeSec, char* buffer, int bufsize);
static CString FormatTime(time_t timeSec);

static CString FormatSpeed(int bytesPerSecond);
static CString FormatSpeed(int64 bytesPerSecond);
static CString FormatSize(int64 fileSize);
static CString FormatBuffer(const char* buf, int len);

Expand Down
Loading

0 comments on commit 6d6d973

Please sign in to comment.