From fa3dbfcc6f9a093f5aa33833121a16561e7cce3d Mon Sep 17 00:00:00 2001 From: Kris Austin Date: Wed, 9 Sep 2026 05:55:19 -0500 Subject: [PATCH] fix: clear 1 warning - report the real error when a Windows G-code export fails (#15582) fix: report the real error when a Windows G-code export fails copy_file built its failure message as "Error: " + errCode. Adding a DWORD to a string literal is pointer arithmetic, not concatenation, so the pointer lands errCode bytes into an 8-byte literal and runs past its end for any code above 7. std::string then calls strlen on it and throws length_error, and the catch(...) in BackgroundSlicingProcess::finalize_gcode replaces the diagnosis with "Unknown error occurred during exporting G-code." Every code a user is likely to hit is past the end: write-protected media is 19, no media 21, a full disk 112, and a destination held open by another program 32. Codes 1 to 7 stay inside the literal and produce a truncated message instead. So the "Maybe the SD card is write locked?" text has not been reachable on Windows since this path was added in #2923. Now that it is reachable, that guess only fits removable media, so it is conditional on m_export_path_on_removable_media. The existing string is untouched and keeps its 23 translations; the fixed-drive case adds one string. --- src/libslic3r/utils.cpp | 2 +- src/slic3r/GUI/BackgroundSlicingProcess.cpp | 4 ++- tests/libslic3r/test_utils.cpp | 36 +++++++++++++++++++++ 3 files changed, 40 insertions(+), 2 deletions(-) diff --git a/src/libslic3r/utils.cpp b/src/libslic3r/utils.cpp index 875c90f6ab..58323b29ce 100644 --- a/src/libslic3r/utils.cpp +++ b/src/libslic3r/utils.cpp @@ -961,7 +961,7 @@ CopyFileResult copy_file(const std::string &from, const std::string &to, std::st BOOL result = CopyFileW(src_wstr, dst_wstr, FALSE); if (!result) { DWORD errCode = GetLastError(); - error_message = "Error: " + errCode; + error_message = "Error: " + std::to_string(errCode); ret = FAIL_COPY_FILE; goto __finished; } diff --git a/src/slic3r/GUI/BackgroundSlicingProcess.cpp b/src/slic3r/GUI/BackgroundSlicingProcess.cpp index 64c52c6e72..f795b41999 100644 --- a/src/slic3r/GUI/BackgroundSlicingProcess.cpp +++ b/src/slic3r/GUI/BackgroundSlicingProcess.cpp @@ -848,7 +848,9 @@ void BackgroundSlicingProcess::finalize_gcode() case CopyFileResult::SUCCESS: break; // no error case CopyFileResult::FAIL_COPY_FILE: throw Slic3r::ExportError(GUI::format( - _L("Copying of the temporary G-code to the output G-code failed. Maybe the SD card is write locked?\nError message: %1%"), + m_export_path_on_removable_media ? + _L("Copying of the temporary G-code to the output G-code failed. Maybe the SD card is write locked?\nError message: %1%") : + _L("Copying of the temporary G-code to the output G-code failed.\nError message: %1%"), error_message)); break; case CopyFileResult::FAIL_FILES_DIFFERENT: diff --git a/tests/libslic3r/test_utils.cpp b/tests/libslic3r/test_utils.cpp index c039069b2a..484438127c 100644 --- a/tests/libslic3r/test_utils.cpp +++ b/tests/libslic3r/test_utils.cpp @@ -2,6 +2,13 @@ #include "libslic3r/Utils.hpp" +#include "test_utils.hpp" + +#include +#include +#include +#include + #ifndef _WIN32 #include // getuid #endif @@ -52,3 +59,32 @@ TEST_CASE("per-user temp root is unchanged on Windows, isolated elsewhere", "[ut REQUIRE_THAT(root, Catch::Matchers::StartsWith(base + "/orcaslicer_")); #endif } + +TEST_CASE("copy_file reports the OS error when the destination cannot be written", "[utils]") { + ScopedTemporaryFile source(".txt"); + { + std::ofstream ofs(source.string(), std::ios::binary); + ofs << "orca"; + } + REQUIRE(boost::filesystem::exists(source.path())); + + // A directory that was never created, so the copy fails on every platform. + const boost::filesystem::path destination = source.path().parent_path() / "orca-missing-dir" / "copy.txt"; + REQUIRE_FALSE(boost::filesystem::exists(destination.parent_path())); + + std::string error_message; + REQUIRE(copy_file(source.string(), destination.string(), error_message) == FAIL_COPY_FILE); + REQUIRE_FALSE(error_message.empty()); + +#ifdef _WIN32 + // The Windows branch formats GetLastError() itself. Writing that as + // "Error: " + errCode adds an integer to a string literal, which indexes into the + // literal instead of appending and runs off its end for any code above 7. + const std::string prefix = "Error: "; + REQUIRE(error_message.rfind(prefix, 0) == 0); + + const std::string code = error_message.substr(prefix.size()); + REQUIRE_FALSE(code.empty()); + REQUIRE(std::all_of(code.begin(), code.end(), [](unsigned char c) { return std::isdigit(c) != 0; })); +#endif // _WIN32 +}