45384 2010-09-08 05:33:51 -0700 Fix ignoring return value warning in case of gcc 4.4.4 2014-12-05 06:55:31 -0800 1 1 1 Unclassified WebKit New Bugs 528+ (Nightly build) PC All RESOLVED FIXED P2 Normal --- 52585 43191 1 pvarga pvarga ap loki ossy zherczeg oldest_to_newest 275771 0 pvarga 2010-09-08 05:33:51 -0700 I got this warning: "ignoring return value of 'size_t fwrite(const void*, size_t, size_t, FILE*)', declared with attribute warn_unused_result". We shouldn't ignore return value of fwrite because it has __attribute__((warn_unused_result)). Patch is coming soon. 275772 1 ossy 2010-09-08 05:36:53 -0700 Nice catch. I didn't run into this, because we use gcc-4.3.2 in Debian Lenny. 275776 2 66885 pvarga 2010-09-08 05:49:16 -0700 Created attachment 66885 proposed patch 275777 3 ossy 2010-09-08 05:51:22 -0700 (In reply to comment #2) > Created an attachment (id=66885) [details] > proposed patch LGTM. 275788 4 66885 kenneth 2010-09-08 06:11:26 -0700 Comment on attachment 66885 proposed patch I would at least add a comment in the ImageDiff case 275794 5 loki 2010-09-08 06:20:38 -0700 I think it would be better using a LIKELY macro instead of an empty if in ImageDiff. It looks better and have the same effect. 275804 6 zherczeg 2010-09-08 06:49:14 -0700 A propose a generic function for GCC: static inline __attribute__((always_inline)) int ignoreIntResult(int x) { return x; } And a define for other platforms: #define ignoreIntResult(x) x It would make clear what the developer wants. 275824 7 pvarga 2010-09-08 07:32:14 -0700 (In reply to comment #5) > I think it would be better using a LIKELY macro instead of an empty if in ImageDiff. It looks better and have the same effect. I think the LIKELY macro can be misleading. (In reply to comment #6) > A propose a generic function for GCC: > > static inline __attribute__((always_inline)) int ignoreIntResult(int x) { return x; } > > And a define for other platforms: > > #define ignoreIntResult(x) x > > It would make clear what the developer wants. To create a new macro for only one case I think it is unnecessary. If there are more then one function with unused result then the idea of use a macro is reasonable. Currently the "empty if" solution is sufficient. 275842 8 ossy 2010-09-08 08:03:49 -0700 (In reply to comment #5) > I think it would be better using a LIKELY macro instead of an empty if in ImageDiff. It looks better and have the same effect. LIKELY macro would be better, but IMHO it wouldn't look better. :) We normally use LIKELY and UNLIKELY macros to give hints to branch predictor. To use them for ignoring a return value is misunderstandable. But I agree, it would be more optimal when you build with -O0, because GCC won't generate useless code for this construction. I think a new IGNORE_RESULT macro would be useful which uses LIKELY. But using LIKELY directly is misunderstandable. (In reply to comment #6) > A propose a generic function for GCC: > > static inline __attribute__((always_inline)) int ignoreIntResult(int x) { return x; } > > And a define for other platforms: > > #define ignoreIntResult(x) x > > It would make clear what the developer wants. It is a good idea, but isn't perfect. For example fwrite has size_t type, other functions can have other type. What do you think about an ignoreResult template similar to in wtf/Unused.h? I mean a construction like this: template<typename T> inline void ignoreResult(T& x) { return x; } #define IGNORE_RESULT(result) UNUSED_PARAM(ignoreResult(result)) 275845 9 66885 ossy 2010-09-08 08:16:24 -0700 Comment on attachment 66885 proposed patch Landed in http://trac.webkit.org/changeset/66984 with comment which Kenneth proposed. I thought it should be landed as soon as possible not to bother developers with build break. But I'm open to discuss about the best and general solution for this bug, so I leave the bug open. 275890 10 ap 2010-09-08 09:46:29 -0700 I think that the most common (and simple) method to ignore this warning is to cast to void: + if (fwrite(data.constData(), 1, data.length(), stdout)) {} - (void)fwrite(data.constData(), 1, data.length(), stdout); 275894 11 ossy 2010-09-08 09:49:05 -0700 (In reply to comment #10) > I think that the most common (and simple) method to ignore this warning is to cast to void: > > + if (fwrite(data.constData(), 1, data.length(), stdout)) {} > - (void)fwrite(data.constData(), 1, data.length(), stdout); We first tried this idea, but unfortunately it doesn't work when __attribute__((warn_unused_result)) is used. :/ Details can be found here: http://gcc.gnu.org/bugzilla/show_bug.cgi?id=25509 275925 12 ap 2010-09-08 10:38:59 -0700 Thanks, I didn't know about that. So, it is considered to be a glibc bug that fwrite is annotated with warn_unused_result. 335477 13 79208 jarred 2011-01-17 13:10:34 -0800 Created attachment 79208 Proposed patch Check the return value instead of disabling a gcc warning flag, to benefit all ports. 335479 14 79208 jarred 2011-01-17 13:12:24 -0800 Comment on attachment 79208 Proposed patch wrong bug, sorry 1052718 15 ossy 2014-12-05 06:55:31 -0800 There is no Qt, no need for any discussion about it. 66885 2010-09-08 05:49:16 -0700 2010-09-08 08:16:24 -0700 proposed patch 0001-proposed.patch text/plain 2871 pvarga RnJvbSA4MzdlMjRjNTU5ZTQ2OGY1MzU3MzZkODEzNTA0MmNjMDkyMGFmYzgzIE1vbiBTZXAgMTcg MDA6MDA6MDAgMjAwMQpGcm9tOiBQZXRlciBWYXJnYSA8cHZhcmdhQGluZi51LXN6ZWdlZC5odT4K RGF0ZTogV2VkLCA4IFNlcCAyMDEwIDE0OjQ3OjA2ICswMjAwClN1YmplY3Q6IFtQQVRDSF0gcHJv cG9zZWQKCi0tLQogV2ViS2l0VG9vbHMvQ2hhbmdlTG9nICAgICAgICAgICAgICAgICAgICAgICAg ICAgICAgfCAgIDE2ICsrKysrKysrKysrKysrKysKIC4uLi9UZXN0TmV0c2NhcGVQbHVnSW4vUGx1 Z2luT2JqZWN0LmNwcCAgICAgICAgICAgIHwgICAgNCArKystCiBXZWJLaXRUb29scy9EdW1wUmVu ZGVyVHJlZS9xdC9JbWFnZURpZmYuY3BwICAgICAgICB8ICAgIDIgKy0KIDMgZmlsZXMgY2hhbmdl ZCwgMjAgaW5zZXJ0aW9ucygrKSwgMiBkZWxldGlvbnMoLSkKCmRpZmYgLS1naXQgYS9XZWJLaXRU b29scy9DaGFuZ2VMb2cgYi9XZWJLaXRUb29scy9DaGFuZ2VMb2cKaW5kZXggNGVlZTA3OC4uZTA1 Y2JmOSAxMDA2NDQKLS0tIGEvV2ViS2l0VG9vbHMvQ2hhbmdlTG9nCisrKyBiL1dlYktpdFRvb2xz L0NoYW5nZUxvZwpAQCAtMSwzICsxLDE5IEBACisyMDEwLTA5LTA4ICBQZXRlciBWYXJnYSAgPHB2 YXJnYUBpbmYudS1zemVnZWQuaHU+CisKKyAgICAgICAgUmV2aWV3ZWQgYnkgTk9CT0RZIChPT1BT ISkuCisKKyAgICAgICAgRml4IGlnbm9yaW5nIHJldHVybiB2YWx1ZSB3YXJuaW5nIGluIGNhc2Ug b2YgZ2NjIDQuNC40CisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNn aT9pZD00NTM4NAorCisgICAgICAgICogRHVtcFJlbmRlclRyZWUvVGVzdE5ldHNjYXBlUGx1Z0lu L1BsdWdpbk9iamVjdC5jcHA6CisgICAgICAgICh0ZXN0UG9zdFVSTEZpbGUpOgorICAgICAgICBJ ZiBmd3JpdGUgaGF2ZSB3cml0dGVuIHplcm8gYnl0ZSB0aGVuIHRoZSB0ZXN0UG9zdFVSTEZpbGUg ZnVuY3Rpb24KKyAgICAgICAgcmV0dXJucyB3aXRoIGZhbHNlIGFzIHRlbXBGaWxlIGNhbid0IGJl IG9wZW5lZC4KKyAgICAgICAgKiBEdW1wUmVuZGVyVHJlZS9xdC9JbWFnZURpZmYuY3BwOgorICAg ICAgICAobWFpbik6CisgICAgICAgIFB1dCBmd3JpdGUgZnVuY3Rpb24gaW50byBhbiBpZiBjb25k aXRpb24gd2l0aG91dCBib2R5IHRvIGF2b2lkCisgICAgICAgIHdhcm5pbmcuIEl0IGlzIHNhZmUg YmVjYXVzZSB0aGlzIGZ1bmN0aW9uIHdyaXRlcyB0byB0aGUgc3Rkb3V0LgorCiAyMDEwLTA5LTA4 ICBTYXRpc2ggU2FtcGF0aCAgPHNhdGlzaEBjaHJvbWl1bS5vcmc+CiAKICAgICAgICAgVW5yZXZp ZXdlZCwgYWRkaW5nIG15c2VsZiB0byBjb21taXR0ZXJzIGxpc3QuCmRpZmYgLS1naXQgYS9XZWJL aXRUb29scy9EdW1wUmVuZGVyVHJlZS9UZXN0TmV0c2NhcGVQbHVnSW4vUGx1Z2luT2JqZWN0LmNw cCBiL1dlYktpdFRvb2xzL0R1bXBSZW5kZXJUcmVlL1Rlc3ROZXRzY2FwZVBsdWdJbi9QbHVnaW5P YmplY3QuY3BwCmluZGV4IDQ0ZmFmYTEuLjUwMDI0MDAgMTAwNjQ0Ci0tLSBhL1dlYktpdFRvb2xz L0R1bXBSZW5kZXJUcmVlL1Rlc3ROZXRzY2FwZVBsdWdJbi9QbHVnaW5PYmplY3QuY3BwCisrKyBi L1dlYktpdFRvb2xzL0R1bXBSZW5kZXJUcmVlL1Rlc3ROZXRzY2FwZVBsdWdJbi9QbHVnaW5PYmpl Y3QuY3BwCkBAIC02OTAsNyArNjkwLDkgQEAgc3RhdGljIGJvb2wgdGVzdFBvc3RVUkxGaWxlKFBs dWdpbk9iamVjdCogb2JqLCBjb25zdCBOUFZhcmlhbnQqIGFyZ3MsIHVpbnQzMl90IGEKICAgICBp ZiAoIXRlbXBGaWxlKQogICAgICAgICByZXR1cm4gZmFsc2U7CiAKLSAgICBmd3JpdGUoY29udGVu dHNTdHJpbmcuVVRGOENoYXJhY3RlcnMsIGNvbnRlbnRzU3RyaW5nLlVURjhMZW5ndGgsIDEsIHRl bXBGaWxlKTsKKyAgICBpZiAoIWZ3cml0ZShjb250ZW50c1N0cmluZy5VVEY4Q2hhcmFjdGVycywg Y29udGVudHNTdHJpbmcuVVRGOExlbmd0aCwgMSwgdGVtcEZpbGUpKQorICAgICAgICByZXR1cm4g ZmFsc2U7CisKICAgICBmY2xvc2UodGVtcEZpbGUpOwogCiAgICAgTlBFcnJvciBlcnJvciA9IGJy b3dzZXItPnBvc3R1cmwob2JqLT5ucHAsIHVybCwgdGFyZ2V0LCBwYXRoU3RyaW5nLlVURjhMZW5n dGgsIHBhdGgsIFRSVUUpOwpkaWZmIC0tZ2l0IGEvV2ViS2l0VG9vbHMvRHVtcFJlbmRlclRyZWUv cXQvSW1hZ2VEaWZmLmNwcCBiL1dlYktpdFRvb2xzL0R1bXBSZW5kZXJUcmVlL3F0L0ltYWdlRGlm Zi5jcHAKaW5kZXggOGVlNjhkYi4uZDJiMGM4MSAxMDA2NDQKLS0tIGEvV2ViS2l0VG9vbHMvRHVt cFJlbmRlclRyZWUvcXQvSW1hZ2VEaWZmLmNwcAorKysgYi9XZWJLaXRUb29scy9EdW1wUmVuZGVy VHJlZS9xdC9JbWFnZURpZmYuY3BwCkBAIC0xMzIsNyArMTMyLDcgQEAgaW50IG1haW4oaW50IGFy Z2MsIGNoYXIqIGFyZ3ZbXSkKICAgICAgICAgICAgICAgICAgICAgYnVmZmVyLmNsb3NlKCk7CiAg ICAgICAgICAgICAgICAgICAgIGNvbnN0IFFCeXRlQXJyYXkgJmRhdGEgPSBidWZmZXIuZGF0YSgp OwogICAgICAgICAgICAgICAgICAgICBwcmludGYoIkNvbnRlbnQtTGVuZ3RoOiAlbHVcbiIsIHN0 YXRpY19jYXN0PHVuc2lnbmVkIGxvbmc+KGRhdGEubGVuZ3RoKCkpKTsKLSAgICAgICAgICAgICAg ICAgICAgZndyaXRlKGRhdGEuY29uc3REYXRhKCksIDEsIGRhdGEubGVuZ3RoKCksIHN0ZG91dCk7 CisgICAgICAgICAgICAgICAgICAgIGlmIChmd3JpdGUoZGF0YS5jb25zdERhdGEoKSwgMSwgZGF0 YS5sZW5ndGgoKSwgc3Rkb3V0KSkge30KIAogICAgICAgICAgICAgICAgICAgICBmcHJpbnRmKHN0 ZG91dCwgImRpZmY6ICUwMS4yZiUlIGZhaWxlZFxuIiwgZGlmZmVyZW5jZSk7CiAgICAgICAgICAg ICAgICAgfQotLSAKMS43LjIuMgoK 79208 2011-01-17 13:10:34 -0800 2011-01-17 13:12:24 -0800 Proposed patch fscanf_break.patch text/plain 1698 jarred ZGlmZiAtLWdpdCBhL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cgYi9Tb3VyY2UvSmF2 YVNjcmlwdENvcmUvQ2hhbmdlTG9nCmluZGV4IGM3YjM1ODkuLjM0YzdlNzQgMTAwNjQ0Ci0tLSBh L1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9DaGFuZ2VMb2cKKysrIGIvU291cmNlL0phdmFTY3JpcHRD b3JlL0NoYW5nZUxvZwpAQCAtMSwzICsxLDE2IEBACisyMDExLTAxLTE3ICBKYXJyZWQgTmljaG9s bHMgIDxqYXJyZWRAc2VuY2hhLmNvbT4KKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9P UFMhKS4KKworICAgICAgICBSRUdSRVNTSU9OKHI3NTcwOSk6IFJldHVybiB2YWx1ZSBvZiBmc2Nh bmYoKSBzaG91bGRuJ3QgYmUgaWdub3JlZC4KKyAgICAgICAgaHR0cHM6Ly9idWdzLndlYmtpdC5v cmcvc2hvd19idWcuY2dpP2lkPTUyNTg1CisgICAgICAgIAorICAgICAgICBnY2MgNC40LjQrIGhh cyB3YXJuX3VudXNlZF92YWx1ZSBhdHRyaWJ1dGUgb24gZnNjYW5mLCBzbyB3ZSBzaG91bGQgY2hl Y2sKKyAgICAgICAgdGhlIHJldHVybiB2YWx1ZSB0byBnZXQgYXJvdW5kIHRoZSBnY2Mgd2Fybmlu ZworCisgICAgICAgICogaml0L0V4ZWN1dGFibGVBbGxvY2F0b3JGaXhlZFZNUG9vbC5jcHA6Cisg ICAgICAgIChKU0M6Om1heWJlTW9kaWZ5Vk1Qb29sU2l6ZSk6CisKIDIwMTEtMDEtMTcgIERhbiBC ZXJuc3RlaW4gIDxtaXR6QGFwcGxlLmNvbT4KIAogICAgICAgICBSdWJiZXItc3RhbXBlZCBieSBN YXJrIFJvd2UuCmRpZmYgLS1naXQgYS9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvaml0L0V4ZWN1dGFi bGVBbGxvY2F0b3JGaXhlZFZNUG9vbC5jcHAgYi9Tb3VyY2UvSmF2YVNjcmlwdENvcmUvaml0L0V4 ZWN1dGFibGVBbGxvY2F0b3JGaXhlZFZNUG9vbC5jcHAKaW5kZXggZGQ3MTFkNC4uZmFiOTFkMiAx MDA2NDQKLS0tIGEvU291cmNlL0phdmFTY3JpcHRDb3JlL2ppdC9FeGVjdXRhYmxlQWxsb2NhdG9y Rml4ZWRWTVBvb2wuY3BwCisrKyBiL1NvdXJjZS9KYXZhU2NyaXB0Q29yZS9qaXQvRXhlY3V0YWJs ZUFsbG9jYXRvckZpeGVkVk1Qb29sLmNwcApAQCAtNDYxLDEwICs0NjEsMTQgQEAgc3RhdGljIHZv aWQgbWF5YmVNb2RpZnlWTVBvb2xTaXplKCkKICAgICAgICAgcmV0dXJuOwogCiAgICAgdW5zaWdu ZWQgb3ZlcmNvbW1pdCA9IDA7Ci0gICAgZnNjYW5mKGZwLCAiJXUiLCAmb3ZlcmNvbW1pdCk7Ci0g ICAgaWYgKG92ZXJjb21taXQgPT0gMSkgewotICAgICAgICB2bVBvb2xTaXplID0gdm1Qb29sU2l6 ZU92ZXJjb21taXQ7Ci0gICAgICAgIGNvYWxlc2NlTGltaXQgPSBjb2FsZXNjZUxpbWl0T3ZlcmNv bW1pdDsKKworICAgIC8vIFdlIGhhdmUgdG8gdXNlIHRoZSByZXR1cm4gdmFsdWUgb2YgZnNjYW5m IHRvIGF2b2lkICJpZ25vcmluZyByZXR1cm4gdmFsdWUiIGdjYyB3YXJuaW5nCisgICAgLy8gU2Vl IGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3dfYnVnLmNnaT9pZD00NTM4NCBmb3IgZGV0YWls cy4KKyAgICBpZiAoZnNjYW5mKGZwLCAiJXUiLCAmb3ZlcmNvbW1pdCkpIHsKKyAgICAgICAgaWYg KG92ZXJjb21taXQgPT0gMSkgeworICAgICAgICAgICAgdm1Qb29sU2l6ZSA9IHZtUG9vbFNpemVP dmVyY29tbWl0OworICAgICAgICAgICAgY29hbGVzY2VMaW1pdCA9IGNvYWxlc2NlTGltaXRPdmVy Y29tbWl0OworICAgICAgICB9CiAgICAgfQogCiAgICAgZmNsb3NlKGZwKTsK