This man page is basically perfect, but I'd still like to first hear some feedback on language, grammar, consistency, before opening this up to the people who know the code.

freebsd_igalic.co added a project: manpages.Sep 13 2023, 11:00 PM

freebsd_igalic.co added a subscriber: manpages.

pstef added a subscriber: pstef.Sep 14 2023, 6:28 AM

pstef added inline comments.

share/man/man9/virtio_endian.9
53	I don't know whether the `the` should be here and above, but currently it's inconsistently used.
58	I think it's conventional to spell this `pass-through` or `passthrough`.

thank you for the review @pstef!
This update addresses your comments.

freebsd_igalic.co added a reviewer: bryanv.Sep 15 2023, 7:01 AM

add more reviewers

fernape added a subscriber: fernape.Sep 21 2023, 2:39 PM

fernape added inline comments.

share/man/man9/virtio_endian.9
65	I would detail the example. We know by the description that the `modern` parameter controls whether the function acts as a passthrough, but we don't know which case is which. Maybe something like This example returns a number with the byte order swapped or something like that.

freebsd_igalic.co updated this revision to Diff 127637.Sep 22 2023, 12:51 PM

freebsd_igalic.co edited the summary of this revision. (Show Details)

freebsd_igalic.co retitled this revision from virtio: start simple with virtio_endian(9) to virtio: start simple, document virtio_endian(9).

freebsd_igalic.co edited the summary of this revision. (Show Details)

i think this is ready to land now.

and if so, it could be MFC'd.

LGTM but I would like to know the opinion of an src committer before this lands.

This revision is now accepted and ready to land.Sep 24 2023, 4:19 PM

yuripv added a subscriber: yuripv.Sep 25 2023, 11:57 PM

yuripv added inline comments.

share/man/man9/Makefile
2331	I don't think this is correct, we already have htole16.9, htole32.9, and htole64.9 links pointing to byteorder(9), and letoh* ones do not exist. You should rather add the functions that man page describes to the NAME section and provide links for those here.
share/man/man9/virtio_endian.9
13	Add functions the man page describes here.
48
51
52
55
56
57	I don't completely understand what this is saying, you first say that the functions do something, but then contradict it as depending on the parameter :) I'd start with something like "if modern is true, these functions return val unchanged. Otherwise the functions ....... act as ....., respectively".

freebsd_igalic.co added inline comments.Sep 26 2023, 10:27 AM

share/man/man9/Makefile
2331	thanks for catching this. I'll correct that, but given these comments, https://reviews.freebsd.org/D41852#957171 it's starting to look like my pursuit is pointless.