Page MenuHomeFreeBSD

More Apple-block function support
AbandonedPublic

Authored by pfg on Jun 20 2015, 3:30 PM.
Tags
None
Referenced Files
Unknown Object (File)
Dec 22 2023, 11:38 PM
Unknown Object (File)
Dec 3 2023, 8:41 AM
Unknown Object (File)
Nov 17 2023, 5:11 AM
Unknown Object (File)
Nov 16 2023, 11:31 AM
Unknown Object (File)
Nov 14 2023, 11:55 AM
Unknown Object (File)
Nov 11 2023, 5:58 AM
Unknown Object (File)
Nov 10 2023, 6:23 AM
Unknown Object (File)
Nov 9 2023, 5:59 AM
Subscribers

Details

Reviewers
theraven
wblock
sson
brooks
Group Reviewers
manpages
Summary

I found this implementation of some missing block functions
in Stacey Son's repo.
I only did some minimal changes and added the functions to the
Symbol map. I haven't done any testing but I thought it would be
good to make them more visible as we should try to get the
support for the block functions and GCD workqueues in
FreeBSD 11.

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Lint Passed
Unit
No Test Coverage

Event Timeline

pfg retitled this revision from to More Apple-block function support.
pfg updated this object.
pfg edited the test plan for this revision. (Show Details)
pfg added reviewers: theraven, sson.

Comments inline.

lib/libc/gen/err.c
80

This doesn't look right. Will it pass the block as the argument when err_exit is called? If not, then bound variables in the block will be undefined values.

lib/libc/gen/fts.c
62

We should move these declarations into block_abi.h and not have them copied into every file that uses them. We should also use __weak_symbol, rather than the expanded form of the attribute.

322

Please can we have an assert for _Block_release != 0 if sp->fts_compar_b != NULL? This will only happen if _Block_copy exists but _Block_release doesn't, which means that something's badly wrong. Ideally, we'd do the _Block_release != 0 check up on line 261, so we never get into this situation in the first place.

1060

Is there a reason not to use qsort_b here?

lib/libc/gen/fts.c
62

I will move them. FWIW, I changed them to the expanded form as a reminder that we still don't have __weak_symbol. Currently __weak is still the only option in cdefs.h.

pfg edited edge metadata.

Move prototypes to the header.
The rest I leave to sson :).

wblock added inline comments.
lib/libc/gen/err.3
173

This should use the function markup, should probably have a comma after the function name for a pause:

function is like
.Fn err_set_exit ,
except it takes the block pointer,

175

Please start new sentences on new lines:

instead of a function pointer.
Note that the

lib/libc/gen/fts.3
69

s/returns/return/

539

This sentence is ambiguous here. The "instead of" can be misinterpreted. It might not be necessary to describe what fts_open does in this sentence, just what fts_open_b does. That reduces the number of things that can be confused. A suggestion follows. Note that I also find the "The blahblah function" needlessly redundant, at least after the first use. I suggest just referring to these and other special words by their name alone, like this sentence refers to fts_open.

.Fn fts_open_b
is like
.Fn fts_open ,
except
.Fa compar
is a block pointer that is passed to
.Xr qsort_b 3 .

A possibly better way to do this would be to say (markup omitted) "fts_open_b is a block-based version of fts_open. It works in the same way, but compar is a block pointer that is passed to qsort_b(3)."

lib/libc/gen/glob.3
341

As above, it might be better to just state the difference and not what the other function does. It might also be better as two sentences, or at least with a comma. In other words:

.Fn glob_b
is a block-based version of
.Fn glob .
The error callback is the block pointer
.Fa errblock .

(No starting "The". Wording might need work. "block-oriented"?)

pfg marked 2 inline comments as done.

Attempt to clean up the markup issues pointed by Warren.

I found it better to use "where" instead of "except".

lib/libc/gen/fts.3
533

This line should be deleted. There is no leading "The" in this form:

"blahblah is"
as opposed to
"The blahblah function is"

541

This last part is still a little ambiguous, although I can see where it might be wanted for comparison. The sentence could be ended immediately after .Xr qsort_b 3. Given the clarity improvement to the first part of the sentence, this part should work either way.

pfg marked 3 inline comments as done.

More polishing by Warren (thanks!).

wblock added a reviewer: wblock.

Looks good for the man pages. Still worth checking with igor and mandoc -Tlint.

This revision is now accepted and ready to land.Jun 23 2015, 3:32 PM
pfg edited edge metadata.
pfg removed rS FreeBSD src repository - subversion as the repository for this revision.

__weak_symbol exists now.

This revision now requires review to proceed.Jun 24 2015, 6:37 PM

Phabricator is still somewhat confused, and shows "Loading..." on some things that probably should have been reviewed.

lib/libc/gen/err.3
31

This should be the date this was changed/added in FreeBSD, unless there is something I've missed. Likewise in the other changed or added man pages.

168

Better to just split the sentence in two:

to perform any necessary cleanup.
Passing a null function pointer for

pfg edited edge metadata.

Note that this is still not fixed, wrt to David's comments so we don't have date for the manpage changes.
(I am also having issues with Phabricator so I am uploading patches)

Fix build issue with __weak_symbol unknown in block_abi.h

pfg added a reviewer: brooks.

This has rusted since the last revision: fts had some changes (mostly just updated prototypes) and glob has been rewritten.
It needs some fresh eyes into it (cc'ing brooks in case he wants to take over).

lib/libc/include/block_abi.h
69

buildworld fails with:
...

> lib/libc_nonshared (obj,depend,all,install)

In file included from /scratch/tmp/pfg/head/lib/libc/stdlib/qsort_r.c:7:
/scratch/tmp/pfg/head/lib/libc/include/block_abi.h:68: error: expected '=', ',', ';', 'asm' or 'attribute' before 'void'
/scratch/tmp/pfg/head/lib/libc/include/block_abi.h:69: error: expected '=', ',', ';', 'asm' or 'attribute' before 'void'
...