[pzstd] Fixes for Windows build

* Add `Portability.h` to fix min/max issues.
* Fix conversion warnings
* Assert that windowLog <= 23, which is currently always the case.
  This could be loosened, but we aren't looking to add new functionality.

Fixes on top of PR #3375 by @eli-schwartz, which added Windows CI for contrib & programs.

diff --git a/contrib/pzstd/Pzstd.cpp b/contrib/pzstd/Pzstd.cpp
index 2c09bda7a2db05d1b97cb104de4a2fa0798c2f05..788cd8fbec3fe619b69412269f55aa1ea8db4510 100644 (file)
--- a/contrib/pzstd/Pzstd.cpp
+++ b/contrib/pzstd/Pzstd.cpp
@@ -10,11 +10,13 @@
 #include "Pzstd.h"
 #include "SkippableFrame.h"
 #include "utils/FileSystem.h"
+#include "utils/Portability.h"
 #include "utils/Range.h"
 #include "utils/ScopeGuard.h"
 #include "utils/ThreadPool.h"
 #include "utils/WorkQueue.h"
 
+#include <algorithm>
 #include <chrono>
 #include <cinttypes>
 #include <cstddef>
@@ -336,6 +338,10 @@ static size_t calculateStep(
     const ZSTD_parameters &params) {
   (void)size;
   (void)numThreads;
+  // Not validated to work correctly for window logs > 23.
+  // It will definitely fail if windowLog + 2 is >= 4GB because
+  // the skippable frame can only store sizes up to 4GB.
+  assert(params.cParams.windowLog <= 23);
   return size_t{1} << (params.cParams.windowLog + 2);
 }
 
@@ -587,7 +593,8 @@ std::uint64_t writeFile(
       // start writing before compression is done because we need to know the
       // compressed size.
       // Wait for the compressed size to be available and write skippable frame
-      SkippableFrame frame(out->size());
+      assert(uint64_t(out->size()) < uint64_t(1) << 32);
+      SkippableFrame frame(uint32_t(out->size()));
       if (!writeData(frame.data(), outputFd)) {
         errorHolder.setError("Failed to write output");
         return bytesWritten;

diff --git a/contrib/pzstd/utils/FileSystem.h b/contrib/pzstd/utils/FileSystem.h
index 8898b03573aee0f366452be200b5f25371cbb098..05c64b879b3d89d024b8b147f61942e476b6db13 100644 (file)
--- a/contrib/pzstd/utils/FileSystem.h
+++ b/contrib/pzstd/utils/FileSystem.h
@@ -8,11 +8,13 @@
  */
 #pragma once
 
+#include "utils/Portability.h"
 #include "utils/Range.h"
 
 #include <sys/stat.h>
 #include <cerrno>
 #include <cstdint>
+#include <limits>
 #include <system_error>
 
 // A small subset of `std::filesystem`.
@@ -82,11 +84,11 @@ inline std::uintmax_t file_size(
     std::error_code& ec) noexcept {
   auto stat = status(path, ec);
   if (ec) {
-    return -1;
+    return std::numeric_limits<uintmax_t>::max();
   }
   if (!is_regular_file(stat)) {
     ec.assign(ENOTSUP, std::generic_category());
-    return -1;
+    return std::numeric_limits<uintmax_t>::max();
   }
   ec.clear();
   return stat.st_size;

diff --git a/contrib/pzstd/utils/Portability.h b/contrib/pzstd/utils/Portability.h
new file mode 100644 (file)
index 0000000..27dbcf7
--- /dev/null
+++ b/contrib/pzstd/utils/Portability.h
@@ -0,0 +1,16 @@
+/*
+ * Copyright (c) 2016-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under both the BSD-style license (found in the
+ * LICENSE file in the root directory of this source tree) and the GPLv2 (found
+ * in the COPYING file in the root directory of this source tree).
+ */
+
+#pragma once
+
+#include <algorithm>
+
+// Required for windows, which defines min/max, but we want the std:: version.
+#undef min
+#undef max

diff --git a/contrib/pzstd/utils/Range.h b/contrib/pzstd/utils/Range.h
index 6a850ad4eaa5bd20718cc9c37d1ac42ac9eb4fa1..c78a8935ac2c661b14adef673ea268430c6fb8cb 100644 (file)
--- a/contrib/pzstd/utils/Range.h
+++ b/contrib/pzstd/utils/Range.h
@@ -14,7 +14,9 @@
 #pragma once
 
 #include "utils/Likely.h"
+#include "utils/Portability.h"
 
+#include <algorithm>
 #include <cstddef>
 #include <cstring>
 #include <stdexcept>