[libFuzzer] Diff 10 - Improve fread error handling.

[kcc]

Are you solving an actual problem or a hypothetical?

[mpividori]

@kcc this changes where suggested by @zturner . hypothetical problem.

[kcc]

I'd like to avoid fixes that increase the code size and fix no observable problem.

[mpividori]

@kcc this changes add a check for ferror() . Previous implementation doesn't check for that possible error.

[kcc]

And I wouldn't care about it, unless you want to add a test.

[mpividori]

@kcc So, if fread() fails, previous implementation will return true. Do you think that is ok?

[kcc]

I don't mind. Nothing in the following workflow will change its behavior.

[mpividori]

@kcc Thanks for your comments. But don't you think this would be better for future uses of ExecuteCommandAndReadOutput() ? To return false if fread fails?

[kcc]

ain't broke don't fix it

Also, if you can't prepare a test for a fix (or don't have time for a test), the fix may not be worth it.
(This is not always true, but frequently).

lib/Fuzzer/FuzzerUtil.cpp

Printf("WARNING: std::thread::hardware_concurrency not well defined for "
       "your platform. Assuming CPU count of 1.\n");
     N = 1;
   }
   return N;
 }
 
 bool ExecuteCommandAndReadOutput(const std::string &Command, std::string *Out) {
   FILE *Pipe = OpenProcessPipe(Command.c_str(), "r");
-  if (!Pipe) return false;
+  if (!Pipe) {
+    Printf("OpenProcessPipe failed with %d.\n", errno);
+    return false;
+  }
+
   char Buff[1024];
   size_t N;
-  while ((N = fread(Buff, 1, sizeof(Buff), Pipe)) > 0)
+  while ((N = fread(Buff, 1, sizeof(Buff), Pipe)) == sizeof(Buff))
     Out->append(Buff, N);
+  if (ferror(Pipe)) {
+    Printf("Failed to read opened process pipe.\n");
+    return false;
+  }
+  // Add last data read.
+  Out->append(Buff, N);
+
   return true;
 }
 
 }  // namespace fuzzer