<div dir="ltr"><div class="gmail_extra"><div class="gmail_quote">On Wed, Nov 13, 2013 at 8:01 PM, Rafael Espíndola <span dir="ltr"><<a href="mailto:rafael.espindola@gmail.com" target="_blank">rafael.espindola@gmail.com</a>></span> wrote:<br>
<blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex">you are not using coff_import_library anywhere in the test. You should<br>
also be able to add a more complete test, no? Can't you, for example,<br>
run llvm-readobj in a "import library"?</blockquote><div><br></div><div>Sorry for the incomplete test. I will update the patch to use coff_import_library in a test.</div><div><br></div><div>As to llvm-readobj, I think we don't have to write a test to run llvm-readobj with an actual file reside on the file system. My test should be sufficient as a unit test for the function that's being modified in the patch. Actually I think this unit test would be better than tests using llvm-readobj because it's more focused on identity_magic's functionality. It's easy to write, understand, and debug.</div>
<div><br></div><blockquote class="gmail_quote" style="margin:0 0 0 .8ex;border-left:1px #ccc solid;padding-left:1ex"><span style="color:rgb(80,0,80)">On 13 November 2013 17:02, Rui Ueyama <</span><a href="mailto:ruiu@google.com">ruiu@google.com</a><span style="color:rgb(80,0,80)">> wrote:</span><div class="HOEnZb">
<div class="h5">
> - added a test<br>
><br>
> Hi Bigcheese,<br>
><br>
> <a href="http://llvm-reviews.chandlerc.com/D2165" target="_blank">http://llvm-reviews.chandlerc.com/D2165</a><br>
><br>
> CHANGE SINCE LAST DIFF<br>
> <a href="http://llvm-reviews.chandlerc.com/D2165?vs=5497&id=5518#toc" target="_blank">http://llvm-reviews.chandlerc.com/D2165?vs=5497&id=5518#toc</a><br>
><br>
> Files:<br>
> include/llvm/Support/FileSystem.h<br>
> lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
> lib/Object/Binary.cpp<br>
> lib/Object/ObjectFile.cpp<br>
> lib/Support/Path.cpp<br>
> unittests/Support/Path.cpp<br>
><br>
> Index: include/llvm/Support/FileSystem.h<br>
> ===================================================================<br>
> --- include/llvm/Support/FileSystem.h<br>
> +++ include/llvm/Support/FileSystem.h<br>
> @@ -238,6 +238,7 @@<br>
> macho_dsym_companion, ///< Mach-O dSYM companion file<br>
> macho_universal_binary, ///< Mach-O universal binary<br>
> coff_object, ///< COFF object file<br>
> + coff_import_library, ///< COFF import library<br>
> pecoff_executable, ///< PECOFF executable file<br>
> windows_resource ///< Windows compiled resource file (.rc)<br>
> };<br>
> Index: lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
> ===================================================================<br>
> --- lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
> +++ lib/ExecutionEngine/RuntimeDyld/RuntimeDyld.cpp<br>
> @@ -584,6 +584,7 @@<br>
> case sys::fs::file_magic::bitcode:<br>
> case sys::fs::file_magic::archive:<br>
> case sys::fs::file_magic::coff_object:<br>
> + case sys::fs::file_magic::coff_import_library:<br>
> case sys::fs::file_magic::pecoff_executable:<br>
> case sys::fs::file_magic::macho_universal_binary:<br>
> case sys::fs::file_magic::windows_resource:<br>
> Index: lib/Object/Binary.cpp<br>
> ===================================================================<br>
> --- lib/Object/Binary.cpp<br>
> +++ lib/Object/Binary.cpp<br>
> @@ -91,6 +91,7 @@<br>
> return object_error::success;<br>
> }<br>
> case sys::fs::file_magic::coff_object:<br>
> + case sys::fs::file_magic::coff_import_library:<br>
> case sys::fs::file_magic::pecoff_executable: {<br>
> OwningPtr<Binary> ret(<br>
> ObjectFile::createCOFFObjectFile(scopedSource.take()));<br>
> Index: lib/Object/ObjectFile.cpp<br>
> ===================================================================<br>
> --- lib/Object/ObjectFile.cpp<br>
> +++ lib/Object/ObjectFile.cpp<br>
> @@ -69,6 +69,7 @@<br>
> case sys::fs::file_magic::macho_dsym_companion:<br>
> return createMachOObjectFile(Object);<br>
> case sys::fs::file_magic::coff_object:<br>
> + case sys::fs::file_magic::coff_import_library:<br>
> case sys::fs::file_magic::pecoff_executable:<br>
> return createCOFFObjectFile(Object);<br>
> }<br>
> Index: lib/Support/Path.cpp<br>
> ===================================================================<br>
> --- lib/Support/Path.cpp<br>
> +++ lib/Support/Path.cpp<br>
> @@ -848,6 +848,10 @@<br>
> return file_magic::unknown;<br>
> switch ((unsigned char)Magic[0]) {<br>
> case 0x00: {<br>
> + // COFF short import library file<br>
> + if (Magic[1] == (char)0x00 && Magic[2] == (char)0xff &&<br>
> + Magic[3] == (char)0xff)<br>
> + return file_magic::coff_import_library;<br>
> // Windows resource file<br>
> const char Expected[] = { 0, 0, 0, 0, '\x20', 0, 0, 0, '\xff' };<br>
> if (Magic.size() >= sizeof(Expected) &&<br>
> Index: unittests/Support/Path.cpp<br>
> ===================================================================<br>
> --- unittests/Support/Path.cpp<br>
> +++ unittests/Support/Path.cpp<br>
> @@ -418,6 +418,7 @@<br>
><br>
> const char archive[] = "!<arch>\x0A";<br>
> const char bitcode[] = "\xde\xc0\x17\x0b";<br>
> +const char coff_import_library[] = "\x00\x00\xff\xff";<br>
> const char elf_relocatable[] = { 0x7f, 'E', 'L', 'F', 1, 2, 1, 0, 0,<br>
> 0, 0, 0, 0, 0, 0, 0, 0, 1 };<br>
> const char macho_universal_binary[] = "\xca\xfe\xba\xbe...\0x00";<br>
</div></div></blockquote></div><br></div></div>