116528 2013-05-21 03:20:21 -0700 [WINCE] Add wtf_bsearch() 2013-05-26 18:25:40 -0700 1 1 1 Unclassified WebKit Web Template Framework 528+ (Nightly build) All All RESOLVED FIXED P2 Normal --- 115807 1 paroga paroga benjamin cmarcelo commit-queue darin oldest_to_newest 891678 0 paroga 2013-05-21 03:20:21 -0700 [WINCE] Add wtf_bsearch() 891681 1 202403 paroga 2013-05-21 03:25:13 -0700 Created attachment 202403 Patch 891683 2 commit-queue 2013-05-21 03:26:36 -0700 Attachment 202403 did not pass style-queue: Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WTF/ChangeLog', u'Source/WTF/wtf/StdLibExtras.h']" exit_code: 1 Source/WTF/wtf/StdLibExtras.h:267: wtf_bsearch is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] Total errors found: 1 in 2 files If any of these errors are false positives, please file a bug against check-webkit-style. 891802 3 202403 darin 2013-05-21 08:52:48 -0700 Comment on attachment 202403 Patch View in context: https://bugs.webkit.org/attachment.cgi?id=202403&action=review >> Source/WTF/wtf/StdLibExtras.h:267 >> +inline void* wtf_bsearch(const void* key, const void* base, size_t count, size_t size, int (*compare)(const void *, const void *)) > > wtf_bsearch is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] This should just be named bsearch. There’s no value to naming it wtf_bsearch and then redirecting with a macro. There may be some reason for this idiom elsewhere, but not here. It’s strange to inline this. It should be just a normal non-inline function, but I suppose you inlined it because you don’t know of a good .cpp file to put it in. 891809 4 paroga 2013-05-21 08:57:11 -0700 (In reply to comment #3) > (From update of attachment 202403 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=202403&action=review > > >> Source/WTF/wtf/StdLibExtras.h:267 > >> +inline void* wtf_bsearch(const void* key, const void* base, size_t count, size_t size, int (*compare)(const void *, const void *)) > > > > wtf_bsearch is incorrectly named. Don't use underscores in your identifier names. [readability/naming/underscores] [4] > > This should just be named bsearch. There’s no value to naming it wtf_bsearch and then redirecting with a macro. There may be some reason for this idiom elsewhere, but not here. There is a problem when it's linked again an application which already provides an implementation of bsearch (not uncommon for WinCE apps). But we can move this problem to the application developer if it's a blocker. > It’s strange to inline this. It should be just a normal non-inline function, but I suppose you inlined it because you don’t know of a good .cpp file to put it in. Yes, but I can add a StdLibExtras.cpp if you like... 893865 5 202403 commit-queue 2013-05-26 18:25:38 -0700 Comment on attachment 202403 Patch Clearing flags on attachment: 202403 Committed r150729: <http://trac.webkit.org/changeset/150729> 893866 6 commit-queue 2013-05-26 18:25:40 -0700 All reviewed patches have been landed. Closing bug. 202403 2013-05-21 03:25:13 -0700 2013-05-26 18:25:38 -0700 Patch bug-116528.patch text/plain 2287 paroga ZGlmZiAtLWdpdCBhL1NvdXJjZS9XVEYvQ2hhbmdlTG9nIGIvU291cmNlL1dURi9DaGFuZ2VMb2cK aW5kZXggNTA2ZjhjYS4uNzY4YmU5NiAxMDA2NDQKLS0tIGEvU291cmNlL1dURi9DaGFuZ2VMb2cK KysrIGIvU291cmNlL1dURi9DaGFuZ2VMb2cKQEAgLTEsMyArMSwxNyBAQAorMjAxMy0wNS0yMSAg UGF0cmljayBHYW5zdGVyZXIgIDxwYXJvZ2FAd2Via2l0Lm9yZz4KKworICAgICAgICBbV0lOQ0Vd IEFkZCB3dGZfYnNlYXJjaCgpCisgICAgICAgIGh0dHBzOi8vYnVncy53ZWJraXQub3JnL3Nob3df YnVnLmNnaT9pZD0xMTY1MjgKKworICAgICAgICBSZXZpZXdlZCBieSBOT0JPRFkgKE9PUFMhKS4K KworICAgICAgICByMTQ5ODMzIGludHJvZHVjZWQgdXNhZ2Ugb2YgOjpic2VhcmNoKCksIHdoaWNo IGRvZXMgbm90IGV4aXN0IG9uIFdpbmR3b3MgQ0UuCisgICAgICAgIEFkZCBvdXIgb3duIGltcGxl bWVudGF0aW9uIG9mIHRoaXMgZnVuY3Rpb24gYXMgd3RmX2JzZWFyY2ggYW5kIGRlZmluZQorICAg ICAgICBic2VhcmNoIGFzIHd0Zl9ic2VhcmNoIHRvIGZpeCBjb21waWxhdGlvbiBvbiBXaW5kd29z IENFLgorCisgICAgICAgICogd3RmL1N0ZExpYkV4dHJhcy5oOgorICAgICAgICAod3RmX2JzZWFy Y2gpOgorCiAyMDEzLTA1LTIwICBaYW4gRG9iZXJzZWsgIDx6ZG9iZXJzZWtAaWdhbGlhLmNvbT4K IAogICAgICAgICBVc2UgJ19fdHlwZW9mX18nIGtleXdvcmQgdG8gYXZvaWQgcHJvYmxlbXMgd2hl biBjb21waWxpbmcgaW4gQysrMTEgc3RhbmRhcmQgY29tcGxpYW5jZQpkaWZmIC0tZ2l0IGEvU291 cmNlL1dURi93dGYvU3RkTGliRXh0cmFzLmggYi9Tb3VyY2UvV1RGL3d0Zi9TdGRMaWJFeHRyYXMu aAppbmRleCA2MDVmOThlLi5lNGRiZTdjIDEwMDY0NAotLS0gYS9Tb3VyY2UvV1RGL3d0Zi9TdGRM aWJFeHRyYXMuaAorKysgYi9Tb3VyY2UvV1RGL3d0Zi9TdGRMaWJFeHRyYXMuaApAQCAtMSw1ICsx LDYgQEAKIC8qCiAgKiBDb3B5cmlnaHQgKEMpIDIwMDggQXBwbGUgSW5jLiBBbGwgUmlnaHRzIFJl c2VydmVkLgorICogQ29weXJpZ2h0IChDKSAyMDEzIFBhdHJpY2sgR2Fuc3RlcmVyIDxwYXJvZ2FA cGFyb2dhLmNvbT4KICAqCiAgKiBSZWRpc3RyaWJ1dGlvbiBhbmQgdXNlIGluIHNvdXJjZSBhbmQg YmluYXJ5IGZvcm1zLCB3aXRoIG9yIHdpdGhvdXQKICAqIG1vZGlmaWNhdGlvbiwgYXJlIHBlcm1p dHRlZCBwcm92aWRlZCB0aGF0IHRoZSBmb2xsb3dpbmcgY29uZGl0aW9ucwpAQCAtMjYxLDYgKzI2 MiwzMSBAQCBpbmxpbmUgQXJyYXlFbGVtZW50VHlwZSogYXBwcm94aW1hdGVCaW5hcnlTZWFyY2go Y29uc3QgQXJyYXlUeXBlJiBhcnJheSwgc2l6ZV90CiAKIH0gLy8gbmFtZXNwYWNlIFdURgogCisj aWYgT1MoV0lOQ0UpCisvLyBXaW5kb3dzIENFIENSVCBoYXMgZG9lcyBub3QgaW1wbGVtZW50IGJz ZWFyY2goKS4KK2lubGluZSB2b2lkKiB3dGZfYnNlYXJjaChjb25zdCB2b2lkKiBrZXksIGNvbnN0 IHZvaWQqIGJhc2UsIHNpemVfdCBjb3VudCwgc2l6ZV90IHNpemUsIGludCAoKmNvbXBhcmUpKGNv bnN0IHZvaWQgKiwgY29uc3Qgdm9pZCAqKSkKK3sKKyAgICBjb25zdCBjaGFyKiBmaXJzdCA9IHN0 YXRpY19jYXN0PGNvbnN0IGNoYXIqPihiYXNlKTsKKworICAgIHdoaWxlIChjb3VudCkgeworICAg ICAgICBzaXplX3QgcG9zID0gKGNvdW50IC0gMSkgPj4gMTsKKyAgICAgICAgY29uc3QgY2hhciog aXRlbSA9IGZpcnN0ICsgcG9zICogc2l6ZTsKKyAgICAgICAgaW50IGNvbXBhcmVSZXN1bHQgPSBj b21wYXJlKGl0ZW0sIGtleSk7CisgICAgICAgIGlmICghY29tcGFyZVJlc3VsdCkKKyAgICAgICAg ICAgIHJldHVybiBjb25zdF9jYXN0PGNoYXIqPihpdGVtKTsKKyAgICAgICAgaWYgKGNvbXBhcmVS ZXN1bHQgPCAwKSB7CisgICAgICAgICAgICBjb3VudCAtPSAocG9zICsgMSk7CisgICAgICAgICAg ICBmaXJzdCArPSAocG9zICsgMSkgKiBzaXplOworICAgICAgICB9IGVsc2UKKyAgICAgICAgICAg IGNvdW50ID0gcG9zOworICAgIH0KKworICAgIHJldHVybiAwOworfQorCisjZGVmaW5lIGJzZWFy Y2goa2V5LCBiYXNlLCBjb3VudCwgc2l6ZSwgY29tcGFyZSkgd3RmX2JzZWFyY2goa2V5LCBiYXNl LCBjb3VudCwgc2l6ZSwgY29tcGFyZSkKKyNlbmRpZgorCiAvLyBUaGlzIHZlcnNpb24gb2YgcGxh Y2VtZW50IG5ldyBvbWl0cyBhIDAgY2hlY2suCiBlbnVtIE5vdE51bGxUYWcgeyBOb3ROdWxsIH07 CiBpbmxpbmUgdm9pZCogb3BlcmF0b3IgbmV3KHNpemVfdCwgTm90TnVsbFRhZywgdm9pZCogbG9j YXRpb24pCg==