Merge pull request #2270 from hzeller/feature-20241002-no-stream-read

If non-mmap: use stdio reading of files instead of std::stream

Author: hzeller

.github/workflows/verible-ci.yml

@@ -462,8 +462,8 @@ jobs:
       uses: actions/cache@v3
       with:
         path: "c:/users/runneradmin/_bazel_runneradmin"
-        key: bazelcache_windows2_${{ steps.cache_timestamp.outputs.time }}
-        restore-keys: bazelcache_windows2_
+        key: bazelcache_windows_${{ steps.cache_timestamp.outputs.time }}
+        restore-keys: bazelcache_windows_
 
     - name: Install dependencies
       run: |

common/util/file_util.cc

@@ -14,13 +14,15 @@
 
 #include "common/util/file_util.h"
 
+#include <fcntl.h>
+
 #include <algorithm>
 #include <cerrno>
+#include <cstdint>
+#include <cstdio>
 #include <cstdlib>
 #include <cstring>
 #include <filesystem>
-#include <fstream>
-#include <iostream>
 #include <memory>
 #include <string>
 #include <system_error>
@@ -34,10 +36,11 @@
 #include "common/util/logging.h"
 
 #ifndef _WIN32
-#include <fcntl.h>
 #include <sys/mman.h>
 #include <sys/stat.h>
 #include <unistd.h>
+#else
+#include <io.h>
 #endif
 
 namespace fs = std::filesystem;
@@ -154,34 +157,35 @@ absl::Status FileExists(const std::string &filename) {
 
 absl::StatusOr<std::string> GetContentAsString(absl::string_view filename) {
   std::string content;
-  std::ifstream fs;
-  std::istream *stream = nullptr;
+  FILE *stream = nullptr;
   const bool use_stdin = IsStdin(filename);
   if (use_stdin) {
-    stream = &std::cin;
+#ifdef _WIN32
+    _setmode(_fileno(stdin), _O_BINARY);  // Work around DOS/Win silliness.
+#endif
+    stream = stdin;
   } else {
     const std::string filename_str = std::string{filename};
     if (absl::Status status = FileExists(filename_str); !status.ok()) {
       return status;  // Bail
     }
-    fs.open(filename_str.c_str());
+    stream = fopen(filename_str.c_str(), "rb");
     std::error_code err;
     const size_t prealloc = fs::file_size(filename_str, err);
     if (err.value() == 0) content.reserve(prealloc);
-    stream = &fs;
   }
-  if (!stream->good()) {
+  if (!stream) {
     return CreateErrorStatusFromErrno(filename, "can't read");
   }
   char buffer[4096];
-  while (stream->good() && !stream->eof()) {
-    stream->read(buffer, sizeof(buffer));
-    content.append(buffer, stream->gcount());
-  }
-
-  // Allow stdin to be reopened for more input.
-  if (use_stdin && std::cin.eof()) std::cin.clear();
-  return std::move(content);
+  int bytes_read;
+  do {
+    bytes_read = fread(buffer, 1, sizeof(buffer), stream);
+    content.append(buffer, bytes_read);
+  } while (bytes_read > 0);
+  fclose(stream);
+
+  return content;
 }
 
 static absl::StatusOr<std::unique_ptr<MemBlock>> AttemptMemMapFile(
@@ -244,11 +248,19 @@ absl::StatusOr<std::unique_ptr<MemBlock>> GetContentAsMemBlock(
 absl::Status SetContents(absl::string_view filename,
                          absl::string_view content) {
   VLOG(1) << __FUNCTION__ << ": Writing file: " << filename;
-  std::ofstream f(std::string(filename).c_str());
-  if (!f.good()) return CreateErrorStatusFromErrno(filename, "can't write.");
-  f << content;
-  f.close();
-  if (!f.good()) return CreateErrorStatusFromErrno(filename, "closing.");
+  FILE *out = fopen(std::string(filename).c_str(), "wb");
+  if (!out) return CreateErrorStatusFromErrno(filename, "can't write.");
+  const int64_t expected_write = content.size();
+  int64_t total_written = 0;
+  while (!content.empty()) {
+    int64_t w = fwrite(content.data(), 1, content.size(), out);
+    total_written += w;
+    content.remove_prefix(w);
+  }
+  const bool written_completely = (total_written == expected_write);
+  if (fclose(out) != 0 || !written_completely) {
+    return CreateErrorStatusFromErrno(filename, "closing.");
+  }
   return absl::OkStatus();
 }
 

verilog/analysis/checkers/token_stream_lint_rule.cc

@@ -75,7 +75,8 @@ void TokenStreamLintRule::HandleSymbol(const verible::Symbol &symbol,
                                        return p->Tag().tag == TK_StringLiteral;
                                      });
   const auto &string_literal = SymbolCastToLeaf(**literal);
-  if (absl::StrContains(string_literal.get().text(), "\\\n")) {
+  if (absl::StrContains(string_literal.get().text(), "\\\n") ||
+      absl::StrContains(string_literal.get().text(), "\\\r")) {
     violations_.insert(LintViolation(string_literal, kMessage, context));
   }
 }

verilog/analysis/checkers/token_stream_lint_rule_test.cc

@@ -56,6 +56,12 @@ TEST(StringLiteralConcatenationTest, FunctionFailures) {
         "\"Humpty Dumpty sat on a wall. \\\nHumpty Dumpty had a great fall.\""},
        ";",
        "\nendmodule"},
+      {"module m;\n",
+       "string tmp=",
+       {kToken,
+        "\"Humpty Dumpty discovers CRLF \\\r\nHumpty Dumpty CRinged.\""},
+       ";",
+       "\nendmodule"},
       {"module m;\n",
        "string tmp=",
        {kToken,

verilog/tools/obfuscator/verilog_obfuscate.cc

@@ -25,6 +25,11 @@
 #include <sstream>  // IWYU pragma: keep  // for ostringstream
 #include <string>   // for string, allocator, etc
 
+#ifdef _WIN32
+#include <fcntl.h>
+#include <io.h>
+#endif
+
 #include "absl/flags/flag.h"
 #include "absl/status/status.h"
 #include "absl/status/statusor.h"
@@ -70,6 +75,12 @@ static constexpr absl::string_view kBuiltinFunctions[] = {
 };
 
 int main(int argc, char **argv) {
+#ifdef _WIN32
+  // stdio: Windows messes with newlines by default. Fix this here.
+  _setmode(_fileno(stdin), _O_BINARY);
+  _setmode(_fileno(stdout), _O_BINARY);
+#endif
+
   const auto usage = absl::StrCat("usage: ", argv[0],
                                   " [options] < original > output\n"
                                   R"(