From: Rob Jones <rob.jones@codethink.co.uk>
To: Steven Whitehouse <swhiteho@redhat.com>,
"Eric W. Biederman" <ebiederm@xmission.com>
Cc: Al Viro <viro@ZenIV.linux.org.uk>,
linux-fsdevel@vger.kernel.org, linux-doc@vger.kernel.org,
linux-kernel@vger.kernel.org, linux-kernel@lists.codethink.co.uk,
ian.molton@codethink.co.uk
Subject: Re: [PATCH] seq_file: Allow private data to be supplied on seq_open
Date: Thu, 07 Aug 2014 15:09:13 +0100 [thread overview]
Message-ID: <53E38889.3000603@codethink.co.uk> (raw)
In-Reply-To: <53E37FDF.50102@redhat.com>
Hi Steve,
On 07/08/14 14:32, Steven Whitehouse wrote:
> Hi,
>
> On 07/08/14 13:58, Rob Jones wrote:
> [snip]
>>
>> On a related subject, Having looked at a few uses of seq_file, I must
>> say that some users seem to make assumptions about the internal
>> workings of the module. Dangerous behaviour as only some behaviours are
>> documented.
>>
>> e.g. The behaviour that "struct seq_file" pointer is stored in
>> file->private_data is documented and can therefore be relied upon but
>> the fact that the output buffer and its size are only defined at the
>> first output (and can therefore be pre-defined and pre-allocated by
>> user code) is not documented and could therefore change without warning.
>>
>> This second behaviour is assumed in, for example, module fs/gfs2/glock.c
>> which could, therefore, stop working properly without warning if the
>> internal behaviour was changed.
>>
> While it is undocumented, it is I understand, how this feature was
> intended to be used, so I think that it is safe to do this in the GFS2
> case. Here is a ref to the thread which explains how it landed up like
> that:
> https://www.redhat.com/archives/cluster-devel/2012-June/msg00000.html
No criticism was intended of that particular piece of code, It has been
there for a couple of years and is, presumably, still working :-)
It was just a general point about things needing to be written down. A
behaviour such as you were relying on can be a very positive thing but
it would be of much greater use if it was written down in the file docs.
I completely missed seq_file_private() because I was looking at the
docs more than the code. If it had been written down in the docs it
would have saved me quite a bit of time, similarly, if the buffer
allocation behaviour was documented, changes to seq_file.c would not be
made that could break your code.
God knows, I'm not a fan of unnecessary documentation but where it's
useful I'm all for it.
>
> Steve.
--
Rob Jones
Codethink Ltd
mailto:rob.jones@codethink.co.uk
tel:+44 161 236 5575
next prev parent reply other threads:[~2014-08-07 14:09 UTC|newest]
Thread overview: 13+ messages / expand[flat|nested] mbox.gz Atom feed top
2014-07-29 17:39 [PATCH] seq_file: Allow private data to be supplied on seq_open Rob Jones
2014-08-06 15:56 ` Rob Jones
2014-08-06 16:02 ` Al Viro
2014-08-06 16:16 ` Rob Jones
2014-08-06 19:14 ` Eric W. Biederman
2014-08-07 12:58 ` Rob Jones
2014-08-07 13:32 ` Steven Whitehouse
2014-08-07 14:09 ` Rob Jones [this message]
2014-08-07 14:16 ` [Linux-kernel] " Rob Jones
2014-08-07 14:22 ` Steven Whitehouse
2014-08-07 14:30 ` Rob Jones
2014-08-06 19:53 ` Al Viro
2014-08-07 1:03 ` Eric W. Biederman
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=53E38889.3000603@codethink.co.uk \
--to=rob.jones@codethink.co.uk \
--cc=ebiederm@xmission.com \
--cc=ian.molton@codethink.co.uk \
--cc=linux-doc@vger.kernel.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@lists.codethink.co.uk \
--cc=linux-kernel@vger.kernel.org \
--cc=swhiteho@redhat.com \
--cc=viro@ZenIV.linux.org.uk \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.