libc: Implement bsearch_s(), document bsearch_b(), and add rudimentary unit tests
Needs ReviewPublic
Actions

Authored by kfv on Sun, Aug 16, 9:37 PM.

Details

Reviewers

dteske
fuz

Summary

Author: Faraz Vahedi <kfv@FreeBSD.org>
Commit: Faraz Vahedi <kfv@FreeBSD.org>

    bsearch.3: Document bsearch_b extension

    Reviewed by:
    Approved by:
    Differential Revision: https://reviews.freebsd.org/D58876

Author: Faraz Vahedi <kfv@FreeBSD.org>
Commit: Faraz Vahedi <kfv@FreeBSD.org>

    libc: Add bsearch_s() from Annex K
    
    Implement bsearch_s() as per §K.3.6.3.2 in C23, first specified
    in C11.  It behaves identically to bsearch(), except the callback
    is called with a third argument, context, which is passed through
    from the caller, and it also performs runtime constraint checking
    on its arguments.

    Reviewed by:
    Approved by:
    Differential Revision: https://reviews.freebsd.org/D58876

Author: Faraz Vahedi <kfv@FreeBSD.org>
Commit: Faraz Vahedi <kfv@FreeBSD.org>

    libc: Add tests for bsearch(), bsearch_b(), and bsearch_s()

    Reviewed by:
    Approved by:
    Differential Revision: https://reviews.freebsd.org/D58876

Diff Detail

Repository

rG FreeBSD src repository

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 75766
Build 72649: arc lint + arc unit

Event Timeline

kfv created this revision.Sun, Aug 16, 9:37 PM

kfv held this revision as a draft.

Herald added subscribers: ziaee, imp. · View Herald TranscriptSun, Aug 16, 9:37 PM

Harbormaster completed remote builds in B75631: Diff 184148.Sun, Aug 16, 9:37 PM

libc: Add bsearch_s() from Annex K

Harbormaster completed remote builds in B75632: Diff 184149.Sun, Aug 16, 9:45 PM

kfv published this revision for review.Sun, Aug 16, 9:46 PM

kfv added reviewers: dteske, fuz.

Looks like a good patch. Could you add unit tests that make sure the constraint handles actually trigger on error?
Please mention in the commit message that this also documents bsearch_b.

The bsearch_b documentation is handled in a separate commit. It displays correctly for me here in Phabricator, but I’ll shortly update the description with the proposed commit messages, as we’ve agreed to use that convention to make reviews easier and clearer. And as for the tests, sure, I’m working on it; I do my best to submit the commit by tonight.

bsearch.3: Document bsearch_b extension
libc: Add bsearch_s() from Annex K
libc: Add tests for bsearch(), bsearch_b(), and bsearch_s()

Harbormaster completed remote builds in B75766: Diff 184396.Wed, Aug 19, 8:09 PM

kfv edited the summary of this revision. (Show Details)Wed, Aug 19, 8:14 PM

kfv retitled this revision from bsearch.3: Document bsearch_b extension to libc: Implement bsearch_s(), document bsearch_b(), and add rudimentary unit tests .

This looks reasonable. Please see comments for some small items to check.

lib/libc/stdlib/bsearch.3
217	Consider adding a HISTORY section documenting when `bsearch_b` and `bsearch_s` were added.
lib/libc/tests/stdlib/bsearch_s_test.c
18	Please use a more descriptive name for this symbol.
31	Please use a more descriptive name for this symbol.