Diff Detail
- Lint
Lint Skipped - Unit
Tests Skipped
Event Timeline
| share/man/man9/seq.9 | ||
|---|---|---|
| 119 ↗ | (On Diff #46766) | s/case reader/case the reader/ |
Possibly a meta question: shouldn't some other manual pages cross reference this manual page?
| share/man/man9/seq.9 | ||
|---|---|---|
| 3 ↗ | (On Diff #46772) | Not needed anymore: https://reviews.freebsd.org/D15264 |
| 125 ↗ | (On Diff #46772) | Missing space before the dot at the end. |
I've run igor and mandoc -Tlint on the manpage and I've found some minor problems.
| share/man/man9/seq.9 | ||
|---|---|---|
| 33 ↗ | (On Diff #46776) | I guess that this is a convention for section 9 manuals that you only put actual function names in the NAME section, right? It looks like it is done in a similar way in nv(9). |
| 89 ↗ | (On Diff #46776) | The Bd macro has to be accompanied by a closing Ed macro. |
| 100 ↗ | (On Diff #46776) | The Bd macro has to be accompanied by a closing Ed macro. |
| 120 ↗ | (On Diff #46776) | mandoc -Tlint reports that the AUTHORS section is out of order. According to mdoc(7) the order should be: ... .Sh STANDARDS .Sh HISTORY .Sh AUTHORS .Sh CAVEATS .Sh BUGS |
| share/man/man9/seq.9 | ||
|---|---|---|
| 120 ↗ | (On Diff #46776) | I don't know if I agree with that. |
| share/man/man9/seq.9 | ||
|---|---|---|
| 120 ↗ | (On Diff #46776) | Well, I understand your argument but on the other hand everyone follows this order as this is what mdoc(7) says. By not putting sections in the correct order we will have mandoc -Tlint always complaining about it. Actually, I always look for BUGS and CAVEATS at the end of the manpage because it's always there. |
This page looks fishy, I'll have to think about.
The livelock caveat is trivially fixable, I just could not be arsed to do it. I'll take care of it this weekend. Meanwhile please wait.
| share/man/man9/seqc.9 | ||
|---|---|---|
| 53 | I think this can be more precise. Something like, seqc allows zero or more readers and zero or one writer to concurrently access an object, providing a consistent snapshot of the object for readers. No mutual exclusion between readers and writers is required, but readers may be starved indefinitely by writers. | |
| 55 | This is mixing the interface with the implementation details. I would separate the two: explain the interface first, and the optionally describe the implementation. | |
| 71 | Presumably amd64? I would not even write "modern" in that case. | |
| 76 | typo: sequence | |
| 106 | Style: should be indented by a tab. | |
| 121 | ... for readers. | |
| 124 | This part is not true, otherwise it would be a serious flaw. The algorithm does not handle multiple concurrent writers. The caller must provide mutual exclusion. | |
| share/man/man9/seqc.9 | ||
|---|---|---|
| 124 | Sorry, I misunderstood. You are right. | |
| share/man/man9/seqc.9 | ||
|---|---|---|
| 56 | The sentence, "The writer functions..." is describing the implementation. I would just get rid of it. | |
| 67 | These two sentences aren't really right. They are conflating fences and locking. The point is that seqc(9) always provides the appropriate fencing, but on amd64 (and some other platforms), the implementation does not require any CPU fences at all. Look at atomic_thread_fence_rel() on amd64 for instance. Consumers of the interface must ensure that two writers are not executing at the same time. It is probably worth stating that explicitly. | |
| 73 | Maybe, "If a writer has started a transaction, this function will spin until the transaction has ended." | |
This mostly looks ok to me, just some final nits.
| share/man/man9/seqc.9 | ||
|---|---|---|
| 66 | "the current sequence number, beginning a read transaction." | |
| 67 | "If a writer is executing a transaction..." | |
| 71 | "... compares a sequence number returned by .Fn seqc_read with the current sequence number." Then the next sentence can be deleted. | |
| 77 | "At the end of a read transaction, .Fn seqc_consistent should be used to determine whether a writer updated the object. If so, the read transaction should be restarted. Otherwise, the read was consistent." | |
| 85 | "In the following example a writer modifies the .Va var1 and .Va var2 fields in .Va obj ." | |
| 96 | These are just two parts of the same example. So I would write, "The following code will obtain a consistent snapshot of .Va var1 and .Var var2:" | |
| share/man/man9/seqc.9 | ||
|---|---|---|
| 122 | It can be noted that a consumer possibly concerned with this can resort to locking after n failed attempts to read. | |
| 126 | This is inaccurate. The counter overflowing at some point is not a necessarily a problem, in fact it can keep overflowing without causing any issues. The problem is when it wraps back to the original value, as stated in the comment. | |
| 128 | A note can be added that the problem is fixable in practice by extending the counter type to 64 bit. | |
| share/man/man9/seqc.9 | ||
|---|---|---|
| 71 | "compares the sequence number with a previously fetched value." | |
| 85 | change -> changes "var1", "var2" and "obj" should be annotated with .Va. | |
| 95 | Same comment as above, with read -> reads. | |
| 97 | "In the case where the..." | |
| 127 | The wording implies that the consumer can use 64-bit integers, but they can't because we use the opaque seqc_t typedef. Maybe, "This could be avoided by extending the interface to allow 64-bit counters." | |