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

Lint
Lint Skipped
Unit
Unit Tests Skipped

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

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

Needs an ending period:

.Xr nv 9 .
127

"allows to" is not something generally said.

family of functions obtains the value of the supplied cookie.
128

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

163

s/during/during the/

Thank you again wblock for your comments! Manpage updated.

share/man/man9/cnv.9
128

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

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

Returned strings, nvlists, descriptors, binaries, or arrays must not be modified
131

"in *an* error state."

136

Typo:
s/rmove/remove/

140

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

144

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

.Fn nvlist_destroy .

and delete the next line,

function.
146

s/with/with the/

152
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

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

155

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

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

200

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

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

130

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

131

Which nvlist?

136

consistency: "removes"

138

the returned memory

149

What is an "element of the supplied cookie"?

154

This should be EXAMPLE (capitals).

oops. updated

share/man/man9/cnv.9
131

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

149

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

Needs an "and"

.Fn nvlist_get_parent ,
and
143

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

154

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

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

Still needs a new line with and.

This revision was automatically updated to reflect the committed changes.