[PATCH] D26956: Initial changes for porting libFuzzer to Windows.

[Zachary Turner via llvm-commits]

zturner added inline comments.


================
Comment at: lib/Fuzzer/FuzzerIOWindows.cpp:29
+bool IsFile(const std::string &Path) {
+  HANDLE FileHandle(
+      CreateFileA(Path.c_str(), 0, FILE_SHARE_READ, NULL, OPEN_EXISTING,
----------------
mpividori wrote:
> zturner wrote:
> > mpividori wrote:
> > > zturner wrote:
> > > > Check the code in `llvm/lib/Support/Windows/Path.inc` for the `status()` function.  There are some subtleties that you should be aware of that this code doesn't handle.  You can probably just rip the code straight from there, changing `W` to `A` so we don't have to worry about widening the paths.
> > > Hi,
> > > Thanks. Yes, I based my implementation in the implementation of `status()` / `getStatus()` from the Support Library.
> > > What I haven't included is the function `isReservedName`, because I assumed that reserved names would return `GetFileType()` different to `FILE_TYPE_DISK`  (probably `FILE_TYPE_CHAR` ?). Do you think I should include check for reserved names? The rest of the code is similar to the implementation of `status()`.
> > Hmm.  The function I'm looking at first calls `GetFileAttributes()`, and then only if it is a reparse point (kinda like a symlink) does it open the handle.  This allows it to "follow the symlink" to get info about the target.  But the advantage of that approach is that `GetFileAttributes()` doesn't actually open the handle -- which can fail in certain scenarios -- unless it has to.
> > 
> > Are we looking at the same function?  The one I'm looking at is `lib/Support/Windows/Path.inc` line 464.
> Hi, in fact the: ` if (attr & FILE_ATTRIBUTE_REPARSE_POINT) ` in `lib/Support/Windows/Path.inc` line 482, doesn't make sense. It does exactly the same when it is a reparse point than when it isn't .....
> If it is not a reparse point, it also uses `CreateFileView` to open a handle...
> I think there are some errors in that code, or the `if` statement is redundant...
Hmm, you are right.  That's some bad code :)

Anyway, for just returning true / false, it's slightly better to only open the file if it's a reparse point.  If it's not a reparse point, you can just return `attr == FILE_ATTRIBUTE_NORMAL` and it will work.


Repository:
  rL LLVM

https://reviews.llvm.org/D26956