189496 2018-09-11 02:22:17 -0700 [Win][Clang][ImageDiff] Fix compilation error and warning of PlatformImageCairo.cpp 2018-09-12 19:05:22 -0700 1 1 1 Unclassified WebKit Tools / Tests WebKit Nightly Build Unspecified Unspecified RESOLVED FIXED InRadar P2 Normal --- 1 fujii fujii achristensen don.olmstead fred.wang lforschler pvollan webkit-bug-importer oldest_to_newest 1458431 0 fujii 2018-09-11 02:22:17 -0700 [Win][Clang][ImageDiff] Fix compilation error and warning of PlatformImageCairo.cpp While trying to build WebKit WinCairo port with the latest Clang (Bug 171618), the following compilation error and warning is reported. > [2/1360] Building CXX object Tools\ImageDiff\CMakeFiles\ImageDiffLib.dir\cairo\PlatformImageCairo.cpp.obj > FAILED: Tools/ImageDiff/CMakeFiles/ImageDiffLib.dir/cairo/PlatformImageCairo.cpp.obj > C:\tools\llvm\bin\clang-cl.exe (...) /FdTools\ImageDiff\CMakeFiles\ImageDiffLib.dir\ -c ..\..\Tools\ImageDiff\cairo\PlatformImageCairo.cpp > ..\..\Tools\ImageDiff\cairo\PlatformImageCairo.cpp(46,22): error: non-constant-expression cannot be narrowed from type 'size_t' (aka 'unsigned long long') to 'unsigned long' in initializer list [-Wc++11-narrowing] > } context { { }, imageSize, 0 }; > ^~~~~~~~~ > ..\..\Tools\ImageDiff\cairo\PlatformImageCairo.cpp(46,22): note: insert an explicit cast to silence this issue > } context { { }, imageSize, 0 }; > ^~~~~~~~~ > static_cast<unsigned long>( ) > ..\..\Tools\ImageDiff\cairo\PlatformImageCairo.cpp(125,61): warning: format specifies type 'unsigned __int64' (aka 'unsigned long long') but the argument has type 'unsigned long' [-Wformat] > fprintf(stdout, "Content-Length: %" FORMAT_SIZE_T "\n", context.writtenBytes); > ~~~~~~~~~~~~~~~~ ^~~~~~~~~~~~~~~~~~~~ > 1 warning and 1 error generated. 1458434 1 349389 fujii 2018-09-11 02:38:47 -0700 Created attachment 349389 Patch 1458488 2 don.olmstead 2018-09-11 09:47:57 -0700 Informal r+ from me. Will look for a formal review. 1458612 3 349389 pvollan 2018-09-11 13:14:10 -0700 Comment on attachment 349389 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349389&action=review > Tools/ImageDiff/cairo/PlatformImageCairo.cpp:40 > - }, &context); > + }, nullptr); Can this parameter be nullptr? I think this is passed to the read function in the first parameter. 1458831 4 349389 fujii 2018-09-11 19:18:09 -0700 Comment on attachment 349389 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349389&action=review >> Tools/ImageDiff/cairo/PlatformImageCairo.cpp:40 >> + }, nullptr); > > Can this parameter be nullptr? I think this is passed to the read function in the first parameter. I don't understand. I removed the unnecessary first argument. This is the spec. https://www.cairographics.org/manual/cairo-PNG-Support.html#cairo-read-func-t 1458835 5 fujii 2018-09-11 19:30:12 -0700 Just for the record. MSVC also reports following warnings in this source file: > 10>c:\webkit\ga\tools\imagediff\cairo\platformimagecairo.cpp(46): warning C4838: conversion from 'size_t' to 'unsigned long' requires a narrowing conversion > 10>c:\webkit\ga\tools\imagediff\cairo\platformimagecairo.cpp(125): warning C4477: 'fprintf' : format string '%Iu' requires an argument of type 'unsigned __int64', but variadic argument 1 has type 'unsigned long' > 10>c:\webkit\ga\tools\imagediff\cairo\platformimagecairo.cpp(125): note: consider using '%lu' in the format string 1458867 6 349389 fujii 2018-09-11 21:50:54 -0700 Comment on attachment 349389 Patch Clearing flags on attachment: 349389 Committed r235929: <https://trac.webkit.org/changeset/235929> 1458868 7 fujii 2018-09-11 21:50:57 -0700 All reviewed patches have been landed. Closing bug. 1458869 8 webkit-bug-importer 2018-09-11 21:51:28 -0700 <rdar://problem/44370981> 1458961 9 349389 fred.wang 2018-09-12 09:01:19 -0700 Comment on attachment 349389 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=349389&action=review >>> Tools/ImageDiff/cairo/PlatformImageCairo.cpp:40 >>> + }, nullptr); >> >> Can this parameter be nullptr? I think this is passed to the read function in the first parameter. > > I don't understand. I removed the unnecessary first argument. > > This is the spec. > https://www.cairographics.org/manual/cairo-PNG-Support.html#cairo-read-func-t Note that this is causing compilation warning now that imageSize is unused. I think you can just write "PlatformImage::createFromStdin(size_t)" now. 1459073 10 fujii 2018-09-12 13:43:14 -0700 Thank you for letting me know. I will fix. 1459223 11 fujii 2018-09-12 19:05:22 -0700 Committed r235965: <https://trac.webkit.org/changeset/235965> 349389 2018-09-11 02:38:47 -0700 2018-09-11 21:50:54 -0700 Patch bug-189496-20180911183846.patch text/plain 2791 fujii U3VidmVyc2lvbiBSZXZpc2lvbjogMjM1ODg2CmRpZmYgLS1naXQgYS9Ub29scy9DaGFuZ2VMb2cg Yi9Ub29scy9DaGFuZ2VMb2cKaW5kZXggN2ExNzk2NTNiYjVjOTNkNjc5OTk5MWE0NmM2MTFmZWM0 ZjMyOWI5ZC4uNTAzMmE4NTk2Y2RjZTBjMTZiYjdlNDA2Y2EwZTE2ODk1ZGExNTEyNSAxMDA2NDQK LS0tIGEvVG9vbHMvQ2hhbmdlTG9nCisrKyBiL1Rvb2xzL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE0 IEBACisyMDE4LTA5LTExICBGdWppaSBIaXJvbm9yaSAgPEhpcm9ub3JpLkZ1amlpQHNvbnkuY29t PgorCisgICAgICAgIFtXaW5dW0NsYW5nXVtJbWFnZURpZmZdIEZpeCBjb21waWxhdGlvbiBlcnJv ciBhbmQgd2FybmluZyBvZiBQbGF0Zm9ybUltYWdlQ2Fpcm8uY3BwCisgICAgICAgIGh0dHBzOi8v YnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD0xODk0OTYKKworICAgICAgICBSZXZpZXdl ZCBieSBOT0JPRFkgKE9PUFMhKS4KKworICAgICAgICAqIEltYWdlRGlmZi9jYWlyby9QbGF0Zm9y bUltYWdlQ2Fpcm8uY3BwOgorICAgICAgICAoSW1hZ2VEaWZmOjpQbGF0Zm9ybUltYWdlOjpjcmVh dGVGcm9tU3RkaW4pOiBSZW1vdmVkIHVubmVjZXNzYXJ5IFJlYWRDb250ZXh0IHN0cnVjdC4KKyAg ICAgICAgKEltYWdlRGlmZjo6UGxhdGZvcm1JbWFnZTo6d3JpdGVBc1BOR1RvU3Rkb3V0KTogVXNl ICclbHUnIGZvcm1hdCB0eXBlIHNwZWNpZmllciBmb3IgdW5zaWduZWQgbG9uZy4KKwogMjAxOC0w OS0xMCAgTWljaGFlbCBTYWJvZmYgIDxtc2Fib2ZmQGFwcGxlLmNvbT4KIAogICAgICAgICBUZXN0 MjYyIGZhaWx1cmUgd2l0aCBOYW1lZCBDYXB0dXJlIEdyb3VwcyAtIHVzaW5nIGEgcmVmZXJlbmNl IGJlZm9yZSB0aGUgZ3JvdXAgaXMgZGVmaW5lZApkaWZmIC0tZ2l0IGEvVG9vbHMvSW1hZ2VEaWZm L2NhaXJvL1BsYXRmb3JtSW1hZ2VDYWlyby5jcHAgYi9Ub29scy9JbWFnZURpZmYvY2Fpcm8vUGxh dGZvcm1JbWFnZUNhaXJvLmNwcAppbmRleCBiOGRjOGYwYTIwZjllOGY5MGFjMWFkNTlkZDE0NjAy Yzc3MzRkNTcxLi45YWQwZmQxZGFjZjJkNjYwMTdmMzU4NTM1Y2VmMDFmNmE4NDNiYWU1IDEwMDY0 NAotLS0gYS9Ub29scy9JbWFnZURpZmYvY2Fpcm8vUGxhdGZvcm1JbWFnZUNhaXJvLmNwcAorKysg Yi9Ub29scy9JbWFnZURpZmYvY2Fpcm8vUGxhdGZvcm1JbWFnZUNhaXJvLmNwcApAQCAtMjksMzAg KzI5LDE1IEBACiAjaW5jbHVkZSA8c3RkaW8uaD4KICNpbmNsdWRlIDxzdGRsaWIuaD4KIAotI2lm ZGVmIF9XSU4zMgotI2RlZmluZSBGT1JNQVRfU0laRV9UICJJdSIKLSNlbHNlCi0jZGVmaW5lIEZP Uk1BVF9TSVpFX1QgInp1IgotI2VuZGlmCi0KIG5hbWVzcGFjZSBJbWFnZURpZmYgewogCiBzdGQ6 OnVuaXF1ZV9wdHI8UGxhdGZvcm1JbWFnZT4gUGxhdGZvcm1JbWFnZTo6Y3JlYXRlRnJvbVN0ZGlu KHNpemVfdCBpbWFnZVNpemUpCiB7Ci0gICAgc3RydWN0IFJlYWRDb250ZXh0IHsKLSAgICAgICAg Y2hhciBidWZmZXJbMjA0OF07Ci0gICAgICAgIHVuc2lnbmVkIGxvbmcgaW5jb21pbmdCeXRlczsK LSAgICAgICAgdW5zaWduZWQgbG9uZyByZWFkQnl0ZXM7Ci0gICAgfSBjb250ZXh0IHsgeyB9LCBp bWFnZVNpemUsIDAgfTsKLQogICAgIGNhaXJvX3N1cmZhY2VfdCogc3VyZmFjZSA9IGNhaXJvX2lt YWdlX3N1cmZhY2VfY3JlYXRlX2Zyb21fcG5nX3N0cmVhbSgKLSAgICAgICAgW10odm9pZCogY2xv c3VyZSwgdW5zaWduZWQgY2hhciogZGF0YSwgdW5zaWduZWQgbGVuZ3RoKSAtPiBjYWlyb19zdGF0 dXNfdCB7Ci0gICAgICAgICAgICBhdXRvJiBjb250ZXh0ID0gKnN0YXRpY19jYXN0PFJlYWRDb250 ZXh0Kj4oY2xvc3VyZSk7Ci0gICAgICAgICAgICBjb250ZXh0LnJlYWRCeXRlcyArPSBsZW5ndGg7 Ci0KKyAgICAgICAgW10odm9pZCosIHVuc2lnbmVkIGNoYXIqIGRhdGEsIHVuc2lnbmVkIGxlbmd0 aCkgLT4gY2Fpcm9fc3RhdHVzX3QgewogICAgICAgICAgICAgc2l6ZV90IHJlYWRCeXRlcyA9IGZy ZWFkKGRhdGEsIDEsIGxlbmd0aCwgc3RkaW4pOwogICAgICAgICAgICAgcmV0dXJuIHJlYWRCeXRl cyA9PSBsZW5ndGggPyBDQUlST19TVEFUVVNfU1VDQ0VTUyA6IENBSVJPX1NUQVRVU19SRUFEX0VS Uk9SOwotICAgICAgICB9LCAmY29udGV4dCk7CisgICAgICAgIH0sIG51bGxwdHIpOwogCiAgICAg aWYgKGNhaXJvX3N1cmZhY2Vfc3RhdHVzKHN1cmZhY2UpICE9IENBSVJPX1NUQVRVU19TVUNDRVNT KSB7CiAgICAgICAgIGNhaXJvX3N1cmZhY2VfZGVzdHJveShzdXJmYWNlKTsKQEAgLTEyMiw3ICsx MDcsNyBAQCB2b2lkIFBsYXRmb3JtSW1hZ2U6OndyaXRlQXNQTkdUb1N0ZG91dCgpCiAgICAgICAg ICAgICBjb250ZXh0LndyaXR0ZW5CeXRlcyArPSBsZW5ndGg7CiAgICAgICAgICAgICByZXR1cm4g Q0FJUk9fU1RBVFVTX1NVQ0NFU1M7CiAgICAgICAgIH0sICZjb250ZXh0KTsKLSAgICBmcHJpbnRm KHN0ZG91dCwgIkNvbnRlbnQtTGVuZ3RoOiAlIiBGT1JNQVRfU0laRV9UICJcbiIsIGNvbnRleHQu d3JpdHRlbkJ5dGVzKTsKKyAgICBmcHJpbnRmKHN0ZG91dCwgIkNvbnRlbnQtTGVuZ3RoOiAlbHVc biIsIGNvbnRleHQud3JpdHRlbkJ5dGVzKTsKICAgICBjYWlyb19zdXJmYWNlX3dyaXRlX3RvX3Bu Z19zdHJlYW0obV9pbWFnZSwKICAgICAgICAgW10odm9pZCosIGNvbnN0IHVuc2lnbmVkIGNoYXIq IGRhdGEsIHVuc2lnbmVkIGxlbmd0aCkgLT4gY2Fpcm9fc3RhdHVzX3QgewogICAgICAgICAgICAg c2l6ZV90IHdyaXR0ZW5CeXRlcyA9IGZ3cml0ZShkYXRhLCAxLCBsZW5ndGgsIHN0ZG91dCk7Cg==