* [PATCH] doc: seq_file: clarify role of *pos in ->next()
@ 2020-09-25 7:14 NeilBrown
2020-09-28 21:19 ` Jonathan Corbet
0 siblings, 1 reply; 2+ messages in thread
From: NeilBrown @ 2020-09-25 7:14 UTC (permalink / raw)
To: Jonathan Corbet; +Cc: linux-doc
[-- Attachment #1: Type: text/plain, Size: 2857 bytes --]
There are behavioural requirements on the seq_file next() function in
terms of how it updates *pos at end-of-file, and these are now enforced
by a warning.
I was recently attempting to justify the reason this was needed, and
couldn't remember the details, and didn't find them in the
documentation.
So I re-read the code until I understood it again, and updated the
documentation to match.
I also enhanced the text about SEQ_START_TOKEN as it seemed potentially
misleading.
Cc: Vasily Averin <vvs@virtuozzo.com>
Signed-off-by: NeilBrown <neilb@suse.de>
---
Documentation/filesystems/seq_file.rst | 20 +++++++++++++++++++-
1 file changed, 19 insertions(+), 1 deletion(-)
diff --git a/Documentation/filesystems/seq_file.rst b/Documentation/filesystems/seq_file.rst
index 7f7ee06b2693..56856481dc8d 100644
--- a/Documentation/filesystems/seq_file.rst
+++ b/Documentation/filesystems/seq_file.rst
@@ -129,7 +129,9 @@ also a special value which can be returned by the start() function
called SEQ_START_TOKEN; it can be used if you wish to instruct your
show() function (described below) to print a header at the top of the
output. SEQ_START_TOKEN should only be used if the offset is zero,
-however.
+however. SEQ_START_TOKEN has no special meaning to the core seq_file
+code. It is provided as a convenience for a start() funciton to
+communicate with the next() and show() functions.
The next function to implement is called, amazingly, next(); its job is to
move the iterator forward to the next position in the sequence. The
@@ -145,6 +147,22 @@ complete. Here's the example version::
return spos;
}
+The next() function should set ``*pos`` to a value that start() can use
+to find the new location in the sequence. When the iterator is being
+stored in the private data area, rather than being reinitialized on each
+start(), it might seem sufficient to simply set ``*pos`` to any non-zero
+value (zero always tells start() to restart the sequence). This is not
+sufficient due to historical problems.
+
+Historically, many next() functions have *not* updated ``*pos`` at
+end-of-file. If the value is then used by start() to initialise the
+iterator, this can result in corner cases where the last entry in the
+sequence is reported twice in the file. In order to discourage this bug
+from being resurrected, the core seq_file code now produces a warning if
+a next() function does not change the value of ``*pos``. Consequently a
+next() function *must* change the value of ``*pos``, and of course must
+set it to a non-zero value.
+
The stop() function closes a session; its job, of course, is to clean
up. If dynamic memory is allocated for the iterator, stop() is the
place to free it; if a lock was taken by start(), stop() must release
--
2.28.0
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 853 bytes --]
^ permalink raw reply related [flat|nested] 2+ messages in thread
* Re: [PATCH] doc: seq_file: clarify role of *pos in ->next()
2020-09-25 7:14 [PATCH] doc: seq_file: clarify role of *pos in ->next() NeilBrown
@ 2020-09-28 21:19 ` Jonathan Corbet
0 siblings, 0 replies; 2+ messages in thread
From: Jonathan Corbet @ 2020-09-28 21:19 UTC (permalink / raw)
To: NeilBrown; +Cc: linux-doc
On Fri, 25 Sep 2020 17:14:42 +1000
NeilBrown <neilb@suse.de> wrote:
> There are behavioural requirements on the seq_file next() function in
> terms of how it updates *pos at end-of-file, and these are now enforced
> by a warning.
>
> I was recently attempting to justify the reason this was needed, and
> couldn't remember the details, and didn't find them in the
> documentation.
>
> So I re-read the code until I understood it again, and updated the
> documentation to match.
>
> I also enhanced the text about SEQ_START_TOKEN as it seemed potentially
> misleading.
>
> Cc: Vasily Averin <vvs@virtuozzo.com>
> Signed-off-by: NeilBrown <neilb@suse.de>
> ---
> Documentation/filesystems/seq_file.rst | 20 +++++++++++++++++++-
> 1 file changed, 19 insertions(+), 1 deletion(-)
Applied, thanks.
jon
^ permalink raw reply [flat|nested] 2+ messages in thread
end of thread, other threads:[~2020-09-28 21:20 UTC | newest]
Thread overview: 2+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-09-25 7:14 [PATCH] doc: seq_file: clarify role of *pos in ->next() NeilBrown
2020-09-28 21:19 ` Jonathan Corbet
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).