Bug 116528 – [WINCE] Add wtf_bsearch()

• Status: RESOLVED
• Resolution: FIXED
• Component: Web Template Framework

[Patrick R. Gansterer]

[WINCE] Add wtf_bsearch()

[Patrick R. Gansterer]

Created attachment 202403 [details]
Patch

[WebKit Commit Bot]

Attachment 202403 [details] 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.

[Darin Adler]

Comment on attachment 202403 [details]
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.

[Patrick R. Gansterer]

(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...

[WebKit Commit Bot]

Comment on attachment 202403 [details]
Patch

Clearing flags on attachment: 202403

Committed r150729: <http://trac.webkit.org/changeset/150729>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.