Bug 116528

Summary: [WINCE] Add wtf_bsearch()
Product: WebKit Reporter: Patrick R. Gansterer <paroga>
Component: Web Template FrameworkAssignee: Patrick R. Gansterer <paroga>
Status: RESOLVED FIXED    
Severity: Normal CC: benjamin, cmarcelo, commit-queue, darin
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: All   
OS: All   
Bug Depends on: 115807    
Bug Blocks:    
Attachments:
Description Flags
Patch none

Description Patrick R. Gansterer 2013-05-21 03:20:21 PDT
[WINCE] Add wtf_bsearch()
Comment 1 Patrick R. Gansterer 2013-05-21 03:25:13 PDT
Created attachment 202403 [details]
Patch
Comment 2 WebKit Commit Bot 2013-05-21 03:26:36 PDT
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.
Comment 3 Darin Adler 2013-05-21 08:52:48 PDT
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.
Comment 4 Patrick R. Gansterer 2013-05-21 08:57:11 PDT
(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...
Comment 5 WebKit Commit Bot 2013-05-26 18:25:38 PDT
Comment on attachment 202403 [details]
Patch

Clearing flags on attachment: 202403

Committed r150729: <http://trac.webkit.org/changeset/150729>
Comment 6 WebKit Commit Bot 2013-05-26 18:25:40 PDT
All reviewed patches have been landed.  Closing bug.