Created attachment 151802 [details] patch to kqueue(2) man page The attached patch adds an EXAMPLE to kqueue(2) man page. I truly think examples are a good ay to improve our documentation. I tried to follow style(9) but I probably missed something
I've created a review for it (but forgot to include the submitter). Here is the link: https://reviews.freebsd.org/D6082 Can the PR submitter address the comments and provide a new patch to this PR? Thanks!
(In reply to Benedict Reuschling from comment #1) I had completely forgotten about this patch... I will have a look at it and send a new version. Thanks!
Created attachment 169639 [details] patch to kqueue(2) man page Patch against the 10.3 version of the manpage. I addressed the comments in phabricator but the one about using EV_ONESHOT and re-arming the event. I don't fully understand what kib@ means. It seems natural to me and easy to understand since only one event is returned and also it is removed after it is consumed. But as I say I don't fully understand what he means.
(In reply to fernando.apesteguia from comment #3) I mean that EV_ONESHOT is excessive when you continuously poll for the same event. It causes useless work for re-registering the event for each iteration. I updated the code by removing the EV_ONESHOT (EV_CLEAR is needed because the vnode events are level-triggered), and also did misc. style cleanups.
Created attachment 169715 [details] Remove EV_ONESHOT
(In reply to Konstantin Belousov from comment #4) OK, I see. Thanks.
Updated the review, integrating kib's latest patch.
A commit references this bug: Author: bcr Date: Sun May 1 18:09:35 UTC 2016 New revision: 298893 URL: https://svnweb.freebsd.org/changeset/base/298893 Log: Provide an example to the kqueue man page, showing a basic usage example. Although it is an untypical example for the use of kqueue, it is better than nothing and should get people started. PR: 196844 Submitted by: fernando.apesteguia@gmail.com Reviewed by: kib Approved by: kib MFC after: 5 days Differential Revision: https://reviews.freebsd.org/D6082 Changes: head/lib/libc/sys/kqueue.2
A commit references this bug: Author: bcr Date: Fri May 6 17:55:11 UTC 2016 New revision: 299190 URL: https://svnweb.freebsd.org/changeset/base/299190 Log: MFC r298893: Provide an example to the kqueue man page, showing a basic usage example. Although it is an untypical example for the use of kqueue, it is better than nothing and should get people started. PR: 196844 Submitted by: fernando.apesteguia@gmail.com Reviewed by: kib Approved by: kib Differential Revision: https://reviews.freebsd.org/D6082 Changes: _U stable/10/ stable/10/lib/libc/sys/kqueue.2
MFC is done. Thanks for your patches, everyone! PR closed.
Re-opening this old PR because of a bug in the example: it evaluates (event.flags & EV_ERROR) for "changelist" argument being "const" and unchanged by system call. The documentation bug needs to be fixed.
(In reply to Eugene Grosbein from comment #11) *stares at kevent(2) for a while* So the first kevent call should be ret = kevent(kq, &event, 1, &tevent, 1, NULL); and the test should be for tevent.flags & EV_ERROR instead of event? Would that correct the problem?
(In reply to PauAmma from comment #12) It depends on what kind of example we want here.
Hi Eugene, You are right. That "event" passed as "changelits" will never change. We could remove the check altogether since we are checking the return value of the syscall. It does not add any value. Or do you have a different thing in mind?
(In reply to Fernando Apesteguía from comment #14) Yes, remove it. Maybe re-add later to the next call that returns events.
Created attachment 233152 [details] Fix EXAMPLE in kqueue.2 mandoc -Tlint and igor clean.
Comment on attachment 233152 [details] Fix EXAMPLE in kqueue.2 >- printf("Something was written in '%s'\en", argv[1]); >- } >+ if (tevent.flags & EV_ERROR) >+ errx(EXIT_FAILURE, "Event error: %s", strerror(event.data)); >+ else >+ printf("Something was written in '%s'\n", argv[1]); \en, since this is a manual page, not a C source file.
Created attachment 233158 [details] Patch with escape sequence properly escaped. Thanks for catching that.
(In reply to Fernando Apesteguía from comment #18) LGTM now.
Cool. FYI, I'm not an src committer. I will need explicit approval from some src developer or member of manpages@. We can do it in this PR, no need to create a review. Cheers.
(In reply to Fernando Apesteguía from comment #20) Approved.
A commit in branch main references this bug: URL: https://cgit.FreeBSD.org/src/commit/?id=e07b0c12ba6435421ceb7dd028402d5cbfc1f1dd commit e07b0c12ba6435421ceb7dd028402d5cbfc1f1dd Author: Fernando Apesteguía <fernape@FreeBSD.org> AuthorDate: 2022-04-11 18:40:28 +0000 Commit: Fernando Apesteguía <fernape@FreeBSD.org> CommitDate: 2022-04-13 06:01:58 +0000 [patch][doc] Fix EXAMPLE in kqueue(2) The error control was not properly implemented. "changelist" is const, hence event.flags is never changed by the syscall. PR: 196844 Reported by: eugen@ Reviewed by: PauAmma <pauamma@gundo.com> Approved by: eugen@ Fixes: 8c231786f01b9f8614e2fe5b47196db1caa7a772 lib/libc/sys/kqueue.2 | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-)
Committed, Thank you both.