diff --git a/src/pull_module/curl_downloader.cpp b/src/pull_module/curl_downloader.cpp index 5c0243550b..ad6f53e18d 100644 --- a/src/pull_module/curl_downloader.cpp +++ b/src/pull_module/curl_downloader.cpp @@ -46,13 +46,45 @@ static void print_download_speed_info(size_t received_size, size_t elapsed_time) printf(" [%.2f %s/s] ", rate, sizeUnits[rate_unit_idx]); } +int computeProgressBarCells(size_t count, size_t max, int barWidth) { + if (max == 0 || barWidth <= 0) { + return 0; + } + const double ratio = static_cast(count) / static_cast(max); + // Written as a positive test so a NaN ratio also lands here rather than falling through. + if (!(ratio > 0.0)) { + return 0; + } + if (ratio >= 1.0) { + return barWidth; + } + return static_cast(ratio * barWidth); +} + static void print_progress(size_t count, size_t max, bool first_run, size_t elapsed_time) { + // A response with no Content-Length reports dltotal == 0, so there is no ratio to show; + // report the running byte count instead. Dividing by max here yielded an infinite ratio + // whose conversion to int is undefined - in practice INT_MIN, which drove the padding + // loop below through roughly 2.1 billion putchar calls on every progress tick. + if (max == 0) { + double received = (double)count; + size_t receivedUnitId = 0; + while (received > 1000 && sizeUnits[receivedUnitId + 1]) { + received /= 1000.0; + receivedUnitId++; + } + printf("\rProgress: %.2f %s downloaded, total size unknown", received, sizeUnits[receivedUnitId]); + print_download_speed_info(count, elapsed_time); + fflush(stdout); + return; + } + float progress = (float)count / max; if (!first_run && progress < 0.01 && count > 0) return; const int bar_width = 50; - int bar_length = progress * bar_width; + const int bar_length = computeProgressBarCells(count, max, bar_width); printf("\rProgress: ["); int i; diff --git a/src/pull_module/curl_downloader.hpp b/src/pull_module/curl_downloader.hpp index 12a9ab39c5..386757a20d 100644 --- a/src/pull_module/curl_downloader.hpp +++ b/src/pull_module/curl_downloader.hpp @@ -14,6 +14,7 @@ // See the License for the specific language governing permissions and // limitations under the License. //***************************************************************************** +#include #include namespace ovms { @@ -23,4 +24,9 @@ Status downloadFileWithCurl(const std::string& url, const std::string& filePath) Status downloadFileWithCurl(const std::string& url, const std::string& filePath, const std::string& authTokenHF); Status fetchUrlToString(const std::string& url, const std::string& authToken, std::string& responseBody); +// Number of filled cells in a barWidth-wide progress bar for count out of max bytes, +// clamped to [0, barWidth]. max == 0 means the server sent no Content-Length, so there is +// no ratio to render and the result is 0. Declared here so the arithmetic can be unit tested. +int computeProgressBarCells(size_t count, size_t max, int barWidth); + } // namespace ovms diff --git a/src/test/pull_hf_model_test.cpp b/src/test/pull_hf_model_test.cpp index 5995159f78..9ba9170b61 100644 --- a/src/test/pull_hf_model_test.cpp +++ b/src/test/pull_hf_model_test.cpp @@ -18,6 +18,7 @@ #include #include #include +#include #include #include #include @@ -47,6 +48,7 @@ #include "src/test/test_file_utils.hpp" #include "src/test/test_with_temp_dir.hpp" #include "src/filesystem/filesystem.hpp" +#include "src/pull_module/curl_downloader.hpp" #include "src/pull_module/hf_pull_model_module.hpp" #include "src/pull_module/libgit2.hpp" #include "src/pull_module/optimum_export.hpp" @@ -378,6 +380,29 @@ ::testing::AssertionResult interruptPosixWorkerAndExpectGracefulExit(pid_t child } // namespace +// A response without Content-Length makes libcurl report dltotal == 0. The progress bar must +// not divide by it: the ratio becomes infinite and converting that to int is undefined, which +// in practice produced INT_MIN and a ~2.1 billion iteration padding loop. +TEST(CurlDownloaderProgressTest, UnknownTotalYieldsNoFilledCells) { + EXPECT_EQ(ovms::computeProgressBarCells(0, 0, 50), 0); + EXPECT_EQ(ovms::computeProgressBarCells(1024, 0, 50), 0); + EXPECT_EQ(ovms::computeProgressBarCells(std::numeric_limits::max(), 0, 50), 0); +} + +TEST(CurlDownloaderProgressTest, FilledCellsTrackRatio) { + EXPECT_EQ(ovms::computeProgressBarCells(0, 100, 50), 0); + EXPECT_EQ(ovms::computeProgressBarCells(50, 100, 50), 25); + EXPECT_EQ(ovms::computeProgressBarCells(100, 100, 50), 50); +} + +// Some servers report more bytes transferred than announced; the bar must stay within its width +// so the padding loop below it always runs a sane number of times. +TEST(CurlDownloaderProgressTest, FilledCellsClampToBarWidth) { + EXPECT_EQ(ovms::computeProgressBarCells(200, 100, 50), 50); + EXPECT_EQ(ovms::computeProgressBarCells(100, 100, 0), 0); + EXPECT_EQ(ovms::computeProgressBarCells(100, 100, -1), 0); +} + // RAII helper class for managing log file lifecycle. // Creates a log file path and automatically removes it on destruction. class LogFileGuard {