[PATCH] D44225: Fix identification of COFF executable files

Rafael Ávila de Espíndola via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Thu Mar 8 04:44:35 PST 2018


rafael added a comment.

I agree. Lets replace the read with MemoryBuffer::getFile. Most callers
will probably use the file anyway and should be using the
identify_magic(StringRef magic) variant.

Cheers,
Rafael

Bob Haarman via Phabricator via llvm-commits
<llvm-commits at lists.llvm.org> writes:

> inglorion added a comment.
> 
> How expensive is it to MemoryBuffer::getFilePath() in all cases? Unless we have reason to believe it is expensive, I would say let's just do that, and then we can remove the code that reads the 32 bytes and detect PE/COFF executables properly.
> 
> I took a look at http://cvsweb.netbsd.org/bsdweb.cgi/src/external/bsd/file/dist/magic/magdir/msdos?rev=1.1.1.10&content-type=text/x-cvsweb-markup&only_with_tag=MAIN to see what file does, and that also looks for the PE header to determine if this is a PE/COFF executable.
> 
> https://reviews.llvm.org/D44225
> 
>  _______________________________________________
> 
> llvm-commits mailing list
>  llvm-commits at lists.llvm.org
>  http://lists.llvm.org/cgi-bin/mailman/listinfo/llvm-commits


https://reviews.llvm.org/D44225





More information about the llvm-commits mailing list