Use standard function to get stdin to binary mode. (#4141)

* Use standard function to get stdin to binary mode.

The fisrt implementation to try to change stdin to binary mode is not
protable.  Using freopen has a lot of implementation defined behaviour,
and the code relied on that working.

Looking into the MSVC documentation, I do not see a protable way of
doing this.  I've implemented a Windows specific method and do nothing
for other systems.  In general, UNIX based systems do make a distinction
between text and binary files, so nothing needs to be done for them.

Fixes #2518

* Split ReadFile into two different functions.

We want to remove the flag parameter.  So the original function has been
replaced with ReadBinaryFile and ReadTextFile.  This should make the
code more readable.

* Change name of CorrectlyReadFile to avoid confusion.

9 files changed, 97 insertions, 41 deletions

diff --git a/tools/as/as.cpp b/tools/as/as.cpp
index f6e96294..c8a44456 100644
--- a/tools/as/as.cpp
+++ b/tools/as/as.cpp
@@ -129,7 +129,7 @@ int main(int argc, char** argv) {
   }
 
   std::vector<char> contents;
-  if (!ReadFile<char>(inFile, "r", &contents)) return 1;
+  if (!ReadTextFile<char>(inFile, &contents)) return 1;
 
   spv_binary binary;
   spv_diagnostic diagnostic = nullptr;
diff --git a/tools/cfg/cfg.cpp b/tools/cfg/cfg.cpp
index ce7f1c22..a93488d5 100644
--- a/tools/cfg/cfg.cpp
+++ b/tools/cfg/cfg.cpp
@@ -104,7 +104,7 @@ int main(int argc, char** argv) {
 
   // Read the input binary.
   std::vector<uint32_t> contents;
-  if (!ReadFile<uint32_t>(inFile, "rb", &contents)) return 1;
+  if (!ReadBinaryFile<uint32_t>(inFile, &contents)) return 1;
   spv_context context = spvContextCreate(kDefaultEnvironment);
   spv_diagnostic diagnostic = nullptr;
 
diff --git a/tools/dis/dis.cpp b/tools/dis/dis.cpp
index bdeeef11..64380db0 100644
--- a/tools/dis/dis.cpp
+++ b/tools/dis/dis.cpp
@@ -180,7 +180,7 @@ int main(int argc, char** argv) {
 
   // Read the input binary.
   std::vector<uint32_t> contents;
-  if (!ReadFile<uint32_t>(inFile, "rb", &contents)) return 1;
+  if (!ReadBinaryFile<uint32_t>(inFile, &contents)) return 1;
 
   // If printing to standard output, then spvBinaryToText should
   // do the printing.  In particular, colour printing on Windows is
diff --git a/tools/fuzz/fuzz.cpp b/tools/fuzz/fuzz.cpp
index 5ee0fb18..3423f6c9 100644
--- a/tools/fuzz/fuzz.cpp
+++ b/tools/fuzz/fuzz.cpp
@@ -568,8 +568,8 @@ bool Fuzz(const spv_target_env& target_env,
         [donor_filename, message_consumer,
          target_env]() -> std::unique_ptr<spvtools::opt::IRContext> {
           std::vector<uint32_t> donor_binary;
-          if (!ReadFile<uint32_t>(donor_filename.c_str(), "rb",
-                                  &donor_binary)) {
+          if (!ReadBinaryFile<uint32_t>(donor_filename.c_str(),
+                                        &donor_binary)) {
             return nullptr;
           }
           return spvtools::BuildModule(target_env, message_consumer,
@@ -672,7 +672,7 @@ int main(int argc, const char** argv) {
   }
 
   std::vector<uint32_t> binary_in;
-  if (!ReadFile<uint32_t>(in_binary_file.c_str(), "rb", &binary_in)) {
+  if (!ReadBinaryFile<uint32_t>(in_binary_file.c_str(), &binary_in)) {
     return 1;
   }
 
diff --git a/tools/io.h b/tools/io.h
index f9cfd9d1..aff9eabd 100644
--- a/tools/io.h
+++ b/tools/io.h
@@ -20,45 +20,101 @@
 #include <cstring>
 #include <vector>
 
-// Appends the content from the file named as |filename| to |data|, assuming
-// each element in the file is of type |T|. The file is opened with the given
-// |mode|. If |filename| is nullptr or "-", reads from the standard input, but
-// reopened with the given mode. If any error occurs, writes error messages to
-// standard error and returns false.
+#if defined(SPIRV_WINDOWS)
+#include <fcntl.h>
+#include <io.h>
+
+#define SET_STDIN_TO_BINARY_MODE() _setmode(_fileno(stdin), O_BINARY);
+#define SET_STDIN_TO_TEXT_MODE() _setmode(_fileno(stdin), O_TEXT);
+#else
+#define SET_STDIN_TO_BINARY_MODE()
+#define SET_STDIN_TO_TEXT_MODE()
+#endif
+
+// Appends the contents of the |file| to |data|, assuming each element in the
+// file is of type |T|.
 template <typename T>
-bool ReadFile(const char* filename, const char* mode, std::vector<T>* data) {
+void ReadFile(FILE* file, std::vector<T>* data) {
+  if (file == nullptr) return;
+
   const int buf_size = 1024;
-  const bool use_file = filename && strcmp("-", filename);
-  if (FILE* fp =
-          (use_file ? fopen(filename, mode) : freopen(nullptr, mode, stdin))) {
-    T buf[buf_size];
-    while (size_t len = fread(buf, sizeof(T), buf_size, fp)) {
-      data->insert(data->end(), buf, buf + len);
-    }
-    if (ftell(fp) == -1L) {
-      if (ferror(fp)) {
-        fprintf(stderr, "error: error reading file '%s'\n", filename);
-        if (use_file) fclose(fp);
-        return false;
-      }
-    } else {
-      if (sizeof(T) != 1 && (ftell(fp) % sizeof(T))) {
-        fprintf(
-            stderr,
-            "error: file size should be a multiple of %zd; file '%s' corrupt\n",
-            sizeof(T), filename);
-        if (use_file) fclose(fp);
-        return false;
-      }
-    }
-    if (use_file) fclose(fp);
-  } else {
+  T buf[buf_size];
+  while (size_t len = fread(buf, sizeof(T), buf_size, file)) {
+    data->insert(data->end(), buf, buf + len);
+  }
+}
+
+// Returns true if |file| has encountered an error opening the file or reading
+// the file as a series of element of type |T|. If there was an error, writes an
+// error message to standard error.
+template <class T>
+bool WasFileCorrectlyRead(FILE* file, const char* filename) {
+  if (file == nullptr) {
     fprintf(stderr, "error: file does not exist '%s'\n", filename);
     return false;
   }
+
+  if (ftell(file) == -1L) {
+    if (ferror(file)) {
+      fprintf(stderr, "error: error reading file '%s'\n", filename);
+      return false;
+    }
+  } else {
+    if (sizeof(T) != 1 && (ftell(file) % sizeof(T))) {
+      fprintf(
+          stderr,
+          "error: file size should be a multiple of %zd; file '%s' corrupt\n",
+          sizeof(T), filename);
+      return false;
+    }
+  }
   return true;
 }
 
+// Appends the contents of the file named |filename| to |data|, assuming
+// each element in the file is of type |T|. The file is opened as a binary file
+// If |filename| is nullptr or "-", reads from the standard input, but
+// reopened as a binary file. If any error occurs, writes error messages to
+// standard error and returns false.
+template <typename T>
+bool ReadBinaryFile(const char* filename, std::vector<T>* data) {
+  const bool use_file = filename && strcmp("-", filename);
+  FILE* fp = nullptr;
+  if (use_file) {
+    fp = fopen(filename, "rb");
+  } else {
+    SET_STDIN_TO_BINARY_MODE();
+    fp = stdin;
+  }
+
+  ReadFile(fp, data);
+  bool succeeded = WasFileCorrectlyRead<T>(fp, filename);
+  if (use_file) fclose(fp);
+  return succeeded;
+}
+
+// Appends the contents of the file named |filename| to |data|, assuming
+// each element in the file is of type |T|. The file is opened as a text file
+// If |filename| is nullptr or "-", reads from the standard input, but
+// reopened as a text file. If any error occurs, writes error messages to
+// standard error and returns false.
+template <typename T>
+bool ReadTextFile(const char* filename, std::vector<T>* data) {
+  const bool use_file = filename && strcmp("-", filename);
+  FILE* fp = nullptr;
+  if (use_file) {
+    fp = fopen(filename, "r");
+  } else {
+    SET_STDIN_TO_TEXT_MODE();
+    fp = stdin;
+  }
+
+  ReadFile(fp, data);
+  bool succeeded = WasFileCorrectlyRead<T>(fp, filename);
+  if (use_file) fclose(fp);
+  return succeeded;
+}
+
 // Writes the given |data| into the file named as |filename| using the given
 // |mode|, assuming |data| is an array of |count| elements of type |T|. If
 // |filename| is nullptr or "-", writes to standard output. If any error occurs,
diff --git a/tools/link/linker.cpp b/tools/link/linker.cpp
index 1956f595..359e8030 100644
--- a/tools/link/linker.cpp
+++ b/tools/link/linker.cpp
@@ -130,7 +130,7 @@ int main(int argc, char** argv) {
 
   std::vector<std::vector<uint32_t>> contents(inFiles.size());
   for (size_t i = 0u; i < inFiles.size(); ++i) {
-    if (!ReadFile<uint32_t>(inFiles[i], "rb", &contents[i])) return 1;
+    if (!ReadBinaryFile<uint32_t>(inFiles[i], &contents[i])) return 1;
   }
 
   const spvtools::MessageConsumer consumer = [](spv_message_level_t level,
diff --git a/tools/opt/opt.cpp b/tools/opt/opt.cpp
index 6999b39a..b9339abe 100644
--- a/tools/opt/opt.cpp
+++ b/tools/opt/opt.cpp
@@ -807,7 +807,7 @@ int main(int argc, const char** argv) {
   }
 
   std::vector<uint32_t> binary;
-  if (!ReadFile<uint32_t>(in_file, "rb", &binary)) {
+  if (!ReadBinaryFile<uint32_t>(in_file, &binary)) {
     return 1;
   }
 
diff --git a/tools/reduce/reduce.cpp b/tools/reduce/reduce.cpp
index 49a5efef..4447b356 100644
--- a/tools/reduce/reduce.cpp
+++ b/tools/reduce/reduce.cpp
@@ -318,7 +318,7 @@ int main(int argc, const char** argv) {
   reducer.SetMessageConsumer(spvtools::utils::CLIMessageConsumer);
 
   std::vector<uint32_t> binary_in;
-  if (!ReadFile<uint32_t>(in_binary_file.c_str(), "rb", &binary_in)) {
+  if (!ReadBinaryFile<uint32_t>(in_binary_file.c_str(), &binary_in)) {
     return 1;
   }
 
diff --git a/tools/val/val.cpp b/tools/val/val.cpp
index 5450023a..21a7d8f4 100644
--- a/tools/val/val.cpp
+++ b/tools/val/val.cpp
@@ -186,7 +186,7 @@ int main(int argc, char** argv) {
   }
 
   std::vector<uint32_t> contents;
-  if (!ReadFile<uint32_t>(inFile, "rb", &contents)) return 1;
+  if (!ReadBinaryFile<uint32_t>(inFile, &contents)) return 1;
 
   spvtools::SpirvTools tools(target_env);
   tools.SetMessageConsumer(spvtools::utils::CLIMessageConsumer);