[PATCH] D27234: [libFuzzer] Diff 4 - Split FuzzerUtil implementation for Posix and Windows.

[Marcos Pividori via Phabricator via llvm-commits]

mpividori added inline comments.


================
Comment at: lib/Fuzzer/FuzzerUtilWindows.cpp:169
+bool ExecuteCommandAndReadOutput(const std::string &Command, std::string *Out) {
+  FILE *Pipe = _popen(Command.c_str(), "r");
+  if (!Pipe) return false;
----------------
zturner wrote:
> This function appears almost entirely the same except `_popen` vs `popen`.  Perhaps this could be hidden behind a function called `OpenProcessPipe` and then `ExecuteCommandAndReadOutput` could be shared.
> 
@zturner thanks. Yes, I agree. I will add that modification.


================
Comment at: lib/Fuzzer/FuzzerUtilWindows.cpp:173-174
+  size_t N;
+  while ((N = fread(Buff, 1, sizeof(Buff), Pipe)) > 0)
+    Out->append(Buff, N);
+  return true;
----------------
zturner wrote:
> This looks wrong to me.  `fread` does not indicate an error by returning a value less than 0, it indicates an error by returning a value different from the requested number of bytes. Then, you check `feof` and `ferr` to determine whether it was an error or an end of file.  This code doesn't seem to handle the case where there was an error reading the process output.
@zturner, Yes you are right. This code was moved from the previous implementation for Posix. Anyway, I will fix this.


Repository:
  rL LLVM

https://reviews.llvm.org/D27234