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.

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;

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;
 

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

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;
   }
 

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,

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,

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;
   }
 

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;
   }
 

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);