Page MenuHomeFreeBSD

Manpage for cnvlist
ClosedPublic

Authored by starak.adam_gmail.com on Jul 19 2016, 12:38 PM.

Details

Reviewers
cem
oshogbo
Group Reviewers
manpages
Commits
rS304910: Introduce cnv man page.
Summary

Hello!
My name is Adam. This year, I participate in Google Summer of Code. I wrote manual page for my API. I would be thankful if you could take a look at it.
Thanks in advance!

Diff Detail

Repository
rS FreeBSD src repository - subversion
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

starak.adam_gmail.com retitled this revision from to Manpage for cnvlist.
starak.adam_gmail.com updated this object.
starak.adam_gmail.com edited the test plan for this revision. (Show Details)
wblock added inline comments.
share/man/man9/cnv.9
118 ↗(On Diff #18552)

This is an if/then sentence, which is halting when read. Swap it around and edit a bit:

The concept of cookies is explained in
123 ↗(On Diff #18552)

Needs an ending period:

.Xr nv 9 .
127 ↗(On Diff #18552)

"allows to" is not something generally said.

family of functions obtains the value of the supplied cookie.
128 ↗(On Diff #18552)

These appear to be copies of the dnvlist man page, so I would suggest waiting for that one and using that wording.

163 ↗(On Diff #18552)

s/during/during the/

Thank you again wblock for your comments! Manpage updated.

share/man/man9/cnv.9
128 ↗(On Diff #18552)

I replaced this sentence with the one you wrote for dnvlist.

starak.adam_gmail.com marked an inline comment as done.

Update after wblock's corrections!

share/man/man9/cnv.9
128 ↗(On Diff #18731)

Needs a serial comma after "binaries" to separate it from "arrays":

Returned strings, nvlists, descriptors, binaries, or arrays must not be modified
131 ↗(On Diff #18731)

"in *an* error state."

136 ↗(On Diff #18731)

Typo:
s/rmove/remove/

140 ↗(On Diff #18731)

The sentence ends on the previous line, so this line should be removed.

144 ↗(On Diff #18731)

As above, change from "nvlist_destroy function." to "nvlist_destroy.":

.Fn nvlist_destroy .

and delete the next line,

function.
146 ↗(On Diff #18731)

s/with/with the/

152 ↗(On Diff #18731)
family of functions removes an element of the supplied cookie and frees all resources

(This has been said in this or the other related man page, possibly more clearly. It might be better to copy that one.)

154 ↗(On Diff #18731)

s/If element/If an element/
s/has wrong/has the wrong/

155 ↗(On Diff #18731)

s/will be/is/

Corrected errors and added example usage

I think you uploaded an empty or reversed diff on accident. Phabricator shows the file as deleted instead of added. :-)

cem added a reviewer: cem.

Have you run igor(1) against your page?

Looks mostly good to me, modulo nits below.

share/man/man9/cnv.9
168 ↗(On Diff #19114)

switch (type) { (space between "switch" and parens (style(9)).

200 ↗(On Diff #19114)

Add a period at the end of the sentence? :)

This revision is now accepted and ready to land.Aug 8 2016, 4:27 PM

It seems a bit strange to refer to close(2) and free(3) from a section 9 (kernel) man page since those functions cannot be called from the kernel. The nv(9) man page already in -current seems to have the same problem. Ideally, the man page would describe the differences between the kernel and userland versions of the APIs.

share/man/man9/cnv.9
127 ↗(On Diff #19114)

"the value associated with the given cookie" like below may be clearer.

130 ↗(On Diff #19114)

It may be clearer to write "must not be modified by the user, since they still belong to the nvlist."

131 ↗(On Diff #19114)

Which nvlist?

136 ↗(On Diff #19114)

consistency: "removes"

138 ↗(On Diff #19114)

the returned memory

149 ↗(On Diff #19114)

What is an "element of the supplied cookie"?

154 ↗(On Diff #19114)

This should be EXAMPLE (capitals).

oops. updated

share/man/man9/cnv.9
131 ↗(On Diff #19114)

The one, on which you used functions mentioned above. (next, get_parent, or nvlist_get_pararr) Otherwise you won't get the cookie.

149 ↗(On Diff #19114)

Well, "element connected with the cookie" might be more clear.

starak.adam_gmail.com edited edge metadata.

updated. Sorry for the last diff.

This revision now requires review to proceed.Aug 9 2016, 11:42 AM
share/man/man9/cnv.9
120 ↗(On Diff #19153)

Needs an "and"

.Fn nvlist_get_parent ,
and
143 ↗(On Diff #19153)

s/with the/with/ --just "with close(2)."

154 ↗(On Diff #19153)

Please avoid "the following", and just use "this". Also, add "the" to "the cnvlist API":

This example demonstrates how to deal with the cnvlist API.
191 ↗(On Diff #19153)

These should be sorted by section number then name. So:

.Xr close 2 ,
​.Xr free 3 ,
.Xr nv 9

Missing that one "and", but otherwise looks good. Thanks!

share/man/man9/cnv.9
120 ↗(On Diff #19153)

Still needs a new line with and.

This revision was automatically updated to reflect the committed changes.