[PATCH] D46813: [llvm-rc] Add missing inputs for tag-icon-cursor.test.

Adrian McCarthy via Phabricator via llvm-commits llvm-commits at lists.llvm.org
Mon May 14 08:28:47 PDT 2018


amccarth added inline comments.


================
Comment at: test/tools/llvm-rc/tag-icon-cursor.test:159
 ; CHECK-NEXT: Data: (
-; CHECK-NEXT:   0000: 00000100 04001010 00000100 20006804  |............ .h.|
+; CHECK-NEXT:   0000: 00000100 04001010 00000100 00006804  |..............h.|
 ; CHECK-NEXT:   0010: 00000300 18180000 01002000 88090000  |.......... .....|
----------------
mstorsjo wrote:
> FWIW, the previous value here seems to be what rc.exe produces, so something is wrong in the current codebase. But improving the test coverage for the current code as it stands probably also is worthwhile, even if there are known issues?
>From the Wikipedia info on the ICO format, this appears to be the bits-per-pixel field (assuming it's an icon and not a cursor).  There's a note that says the field may be 0 (in most cases) because the bits-per-pixel can (usually) be inferred.

My guess is the source file has 0 and llvm-rc copies it verbatim while rc.exe infers the BPP and sets it explicitly.  In theory, it should work either way.

Maybe add a comment here so that if we ever do match rc.exe, nobody has to sweat the details to make the test pass.


Repository:
  rL LLVM

https://reviews.llvm.org/D46813





More information about the llvm-commits mailing list