Improves

src/node_main.cc

@@ -21,78 +21,89 @@
 
 #include "node.h"
 #include <cstdio>
+#include <memory>
+#include <vector>
 
 #ifdef _WIN32
 #include <windows.h>
 #include <VersionHelpers.h>
 #include <WinError.h>
 
-#define SKIP_CHECK_VAR "NODE_SKIP_PLATFORM_CHECK"
-#define SKIP_CHECK_VALUE "1"
-#define SKIP_CHECK_STRLEN (sizeof(SKIP_CHECK_VALUE) - 1)
+namespace {
+constexpr const char* SKIP_CHECK_VAR = "NODE_SKIP_PLATFORM_CHECK";
+constexpr const char* SKIP_CHECK_VALUE = "1";
+constexpr size_t SKIP_CHECK_STRLEN = sizeof(SKIP_CHECK_VALUE) - 1;
 
-int wmain(int argc, wchar_t* wargv[]) {
-  // Windows Server 2012 (not R2) is supported until 10/10/2023, so we allow it
-  // to run in the experimental support tier.
+// Custom error codes
+enum class NodeErrorCode {
+  SUCCESS = 0,
+  PLATFORM_CHECK_FAILED = ERROR_EXE_MACHINE_TYPE_MISMATCH,
+  ARGUMENT_CONVERSION_FAILED = 2
+};
+
+bool CheckWindowsVersion() {
   char buf[SKIP_CHECK_STRLEN + 1];
-  if (!IsWindows10OrGreater() &&
-      (GetEnvironmentVariableA(SKIP_CHECK_VAR, buf, sizeof(buf)) !=
-           SKIP_CHECK_STRLEN ||
-       strncmp(buf, SKIP_CHECK_VALUE, SKIP_CHECK_STRLEN) != 0)) {
-    fprintf(stderr,
-            "Node.js is only supported on Windows 10, Windows "
-            "Server 2016, or higher.\n"
-            "Setting the " SKIP_CHECK_VAR " environment variable "
-            "to 1 skips this\ncheck, but Node.js might not execute "
-            "correctly. Any issues encountered on\nunsupported "
-            "platforms will not be fixed.");
-    exit(ERROR_EXE_MACHINE_TYPE_MISMATCH);
+  if (IsWindows10OrGreater()) return true;
+
+  return (GetEnvironmentVariableA(SKIP_CHECK_VAR, buf, sizeof(buf)) == SKIP_CHECK_STRLEN &&
+          strncmp(buf, SKIP_CHECK_VALUE, SKIP_CHECK_STRLEN) == 0);
+}
+
+void PrintPlatformError() {
+  fprintf(stderr,
+          "Node.js is only supported on Windows 10, Windows Server 2016, or higher.\n"
+          "Setting the %s environment variable to 1 skips this check, but Node.js\n"
+          "might not execute correctly. Any issues encountered on unsupported\n"
+          "platforms will not be fixed.\n",
+          SKIP_CHECK_VAR);
+}
+
+std::unique_ptr<char[]> ConvertWideToUTF8(const wchar_t* wide_str) {
+  // Compute the size of the required buffer
+  DWORD size = WideCharToMultiByte(CP_UTF8, 0, wide_str, -1, nullptr, 0, nullptr, nullptr);
+  if (size == 0) {
+    fprintf(stderr, "Error: Could not determine UTF-8 buffer size for argument conversion.\n");
+    exit(static_cast<int>(NodeErrorCode::ARGUMENT_CONVERSION_FAILED));
+  }
+
+  // Allocate and convert
+  auto utf8_str = std::make_unique<char[]>(size);
+  DWORD result = WideCharToMultiByte(CP_UTF8, 0, wide_str, -1, utf8_str.get(), size, nullptr, nullptr);
+  if (result == 0) {
+    fprintf(stderr, "Error: Failed to convert command line argument to UTF-8.\n");
+    exit(static_cast<int>(NodeErrorCode::ARGUMENT_CONVERSION_FAILED));
   }
 
-  // Convert argv to UTF8
-  char** argv = new char*[argc + 1];
+  return utf8_str;
+}
+}  // namespace
+
+int wmain(int argc, wchar_t* wargv[]) {
+  // Check Windows version compatibility
+  if (!CheckWindowsVersion()) {
+    PrintPlatformError();
+    return static_cast<int>(NodeErrorCode::PLATFORM_CHECK_FAILED);
+  }
+
+  // Convert argv to UTF8 using RAII
+  std::vector<std::unique_ptr<char[]>> arg_storage;
+  std::vector<char*> argv;
+
+  arg_storage.reserve(argc);
+  argv.reserve(argc + 1);
+
   for (int i = 0; i < argc; i++) {
-    // Compute the size of the required buffer
-    DWORD size = WideCharToMultiByte(CP_UTF8,
-                                     0,
-                                     wargv[i],
-                                     -1,
-                                     nullptr,
-                                     0,
-                                     nullptr,
-                                     nullptr);
-    if (size == 0) {
-      // This should never happen.
-      fprintf(stderr, "Could not convert arguments to utf8.");
-      // TODO(joyeecheung): should be ExitCode::kInvalidCommandLineArgument,
-      // but we are not ready to expose that to node.h yet.
-      exit(1);
-    }
-    // Do the actual conversion
-    argv[i] = new char[size];
-    DWORD result = WideCharToMultiByte(CP_UTF8,
-                                       0,
-                                       wargv[i],
-                                       -1,
-                                       argv[i],
-                                       size,
-                                       nullptr,
-                                       nullptr);
-    if (result == 0) {
-      // This should never happen.
-      fprintf(stderr, "Could not convert arguments to utf8.");
-      // TODO(joyeecheung): should be ExitCode::kInvalidCommandLineArgument,
-      // but we are not ready to expose that to node.h yet.
-      exit(1);
-    }
+    auto utf8_arg = ConvertWideToUTF8(wargv[i]);
+    argv.push_back(utf8_arg.get());
+    arg_storage.push_back(std::move(utf8_arg));
   }
-  argv[argc] = nullptr;
-  // Now that conversion is done, we can finally start.
-  return node::Start(argc, argv);
+  argv.push_back(nullptr);
+
+  // Start Node.js
+  return node::Start(argc, argv.data());
 }
 #else
 // UNIX
-
 int main(int argc, char* argv[]) {
   return node::Start(argc, argv);
 }

src/string_bytes.h

@@ -24,86 +24,138 @@
 
 #if defined(NODE_WANT_INTERNALS) && NODE_WANT_INTERNALS
 
-// Decodes a v8::Local<v8::String> or Buffer to a raw char*
-
 #include "v8.h"
 #include "env-inl.h"
-
 #include <string>
+#include <string_view>
+#include <memory>
+#include <optional>
 
 namespace node {
 
+// Forward declarations
+enum class Encoding : uint8_t {
+  ASCII,
+  UTF8,
+  BASE64,
+  UCS2,
+  BINARY,
+  HEX,
+  BUFFER,
+  LATIN1 = BINARY
+};
+
+/**
+ * @brief Handles conversion between V8 strings/buffers and raw bytes
+ * 
+ * This class provides utilities for encoding and decoding between V8's string
+ * representations and various byte encodings. It handles proper memory management
+ * and encoding validation.
+ */
 class StringBytes {
  public:
+  /**
+   * @brief Inline decoder for efficient string-to-bytes conversion
+   * 
+   * Uses stack buffer when possible, automatically falls back to heap for larger strings.
+   * Provides RAII-compliant memory management.
+   */
   class InlineDecoder : public MaybeStackBuffer<char> {
    public:
+    /**
+     * @brief Decodes a V8 string to bytes using specified encoding
+     * 
+     * @param env The V8 environment
+     * @param string The source string to decode
+     * @param enc The target encoding
+     * @return v8::Maybe<void> Success/failure of the operation
+     */
     inline v8::Maybe<void> Decode(Environment* env,
-                                  v8::Local<v8::String> string,
-                                  enum encoding enc) {
+                                 v8::Local<v8::String> string,
+                                 Encoding enc) {
       size_t storage;
       if (!StringBytes::StorageSize(env->isolate(), string, enc).To(&storage))
         return v8::Nothing<void>();
+
       AllocateSufficientStorage(storage);
       const size_t length =
           StringBytes::Write(env->isolate(), out(), storage, string, enc);
 
-      // No zero terminator is included when using this method.
       SetLength(length);
       return v8::JustVoid();
     }
 
-    inline size_t size() const { return length(); }
+    [[nodiscard]] inline size_t size() const noexcept { return length(); }
   };
 
-  // Fast, but can be 2 bytes oversized for Base64, and
-  // as much as triple UTF-8 strings <= 65536 chars in length
-  static v8::Maybe<size_t> StorageSize(v8::Isolate* isolate,
-                                       v8::Local<v8::Value> val,
-                                       enum encoding enc);
-
-  // Precise byte count, but slightly slower for Base64 and
-  // very much slower for UTF-8
-  static v8::Maybe<size_t> Size(v8::Isolate* isolate,
-                                v8::Local<v8::Value> val,
-                                enum encoding enc);
-
-  // Write the bytes from the string or buffer into the char*
-  // returns the number of bytes written, which will always be
-  // <= buflen.  Use StorageSize/Size first to know how much
-  // memory to allocate.
+  /**
+   * @brief Calculates required storage size for encoding conversion
+   * 
+   * Fast but may overestimate by up to:
+   * - 2 bytes for Base64
+   * - 3x for UTF-8 strings <= 65536 chars
+   */
+  [[nodiscard]] static v8::Maybe<size_t> StorageSize(
+      v8::Isolate* isolate,
+      v8::Local<v8::Value> val,
+      Encoding enc) noexcept;
+
+  /**
+   * @brief Calculates exact byte count needed
+   * 
+   * More precise but slower than StorageSize, especially for:
+   * - Base64 encoding
+   * - UTF-8 encoding
+   */
+  [[nodiscard]] static v8::Maybe<size_t> Size(
+      v8::Isolate* isolate,
+      v8::Local<v8::Value> val,
+      Encoding enc) noexcept;
+
+  /**
+   * @brief Writes bytes from string/buffer to char buffer
+   * 
+   * @return Actual number of bytes written (always <= buflen)
+   */
   static size_t Write(v8::Isolate* isolate,
-                      char* buf,
-                      size_t buflen,
-                      v8::Local<v8::Value> val,
-                      enum encoding enc);
-
-  // Take the bytes in the src, and turn it into a Buffer or String.
-  static v8::MaybeLocal<v8::Value> Encode(v8::Isolate* isolate,
-                                          const char* buf,
-                                          size_t buflen,
-                                          enum encoding encoding,
-                                          v8::Local<v8::Value>* error);
-
-  // Warning: This reverses endianness on BE platforms, even though the
-  // signature using uint16_t implies that it should not.
-  // However, the brokenness is already public API and can't therefore
-  // be changed easily.
-  static v8::MaybeLocal<v8::Value> Encode(v8::Isolate* isolate,
-                                          const uint16_t* buf,
-                                          size_t buflen,
-                                          v8::Local<v8::Value>* error);
-
-  static v8::MaybeLocal<v8::Value> Encode(v8::Isolate* isolate,
-                                          const char* buf,
-                                          enum encoding encoding,
-                                          v8::Local<v8::Value>* error);
+                     char* buf,
+                     size_t buflen,
+                     v8::Local<v8::Value> val,
+                     Encoding enc);
+
+  /**
+   * @brief Encodes raw bytes into V8 Buffer or String
+   */
+  [[nodiscard]] static v8::MaybeLocal<v8::Value> Encode(
+      v8::Isolate* isolate,
+      std::string_view buf,
+      Encoding encoding,
+      v8::Local<v8::Value>* error);
+
+  /**
+   * @brief Encodes UTF-16 data into V8 value
+   * @warning Reverses endianness on BE platforms
+   */
+  [[nodiscard]] static v8::MaybeLocal<v8::Value> Encode(
+      v8::Isolate* isolate,
+      const uint16_t* buf,
+      size_t buflen,
+      v8::Local<v8::Value>* error);
 
  private:
   static size_t WriteUCS2(v8::Isolate* isolate,
-                          char* buf,
-                          size_t buflen,
-                          v8::Local<v8::String> str,
-                          int flags);
+                         char* buf,
+                         size_t buflen,
+                         v8::Local<v8::String> str,
+                         int flags);
+
+  // Prevent instantiation - this is a utility class
+  StringBytes() = delete;
+  ~StringBytes() = delete;
+  StringBytes(const StringBytes&) = delete;
+  StringBytes& operator=(const StringBytes&) = delete;
+  StringBytes(StringBytes&&) = delete;
+  StringBytes& operator=(StringBytes&&) = delete;
 };
 
 }  // namespace node