All of lore.kernel.org
 help / color / mirror / Atom feed
From: Rob Jones <rob.jones@codethink.co.uk>
To: Randy Dunlap <rdunlap@infradead.org>,
	Andrew Morton <akpm@linux-foundation.org>
Cc: viro@zeniv.linux.org.uk, linux-doc@vger.kernel.org,
	linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org,
	linux-kernel@codethink.co.uk, keescook@chromium.org,
	penguin-kernel@I-love.SAKURA.ne.jp
Subject: Re: [PATCH RESUBMIT 1/2] fs/seq_file: Create new function seq_open_init()
Date: Thu, 25 Sep 2014 18:39:29 +0100	[thread overview]
Message-ID: <54245351.1010701@codethink.co.uk> (raw)
In-Reply-To: <54242B74.3040005@infradead.org>


On 25/09/14 15:49, Randy Dunlap wrote:
> On 09/25/14 02:10, Rob Jones wrote:
>>
>>
>> On 24/09/14 22:39, Andrew Morton wrote:
>>> On Wed, 24 Sep 2014 12:15:55 +0100 Rob Jones <rob.jones@codethink.co.uk> wrote:
>>>
>>>> Add a new function to help reduce boilerplate code.
>>>>
>>>> This is a wrapper function for seq_open() that will simplify the code in a
>>>> significant number of cases where seq_open() is currently called.
>>>>
>>>> It's first use is in __seq_open_private(), thereby recovering most of
>>>> the code space used by the new function.
>>>
>>> It would be nice to include one or more of the conversions in this patch
>>> series so we can see what the effects look like.
>>
>> There are certainly lots of candidates around. However, I thought that
>> the change to __seq_open_private() already gave a good illustration of
>> the level of savings to be made, in that it more or less made the new
>> function "self financing".
>>
>>>
>>>> --- a/fs/seq_file.c
>>>> +++ b/fs/seq_file.c
>>>> @@ -639,28 +639,38 @@ int seq_release_private(struct inode *inode, struct file *file)
>>>>    }
>>>>    EXPORT_SYMBOL(seq_release_private);
>>>>
>>>> +int seq_open_init(struct file *f, const struct seq_operations *ops, void *p)
>>>> +{
>>>> +    struct seq_file *s;
>>>> +    int rc;
>>>> +
>>>> +    rc = seq_open(f, ops);
>>>> +    if (rc)
>>>> +        return rc;
>>>> +
>>>> +    s = f->private_data;
>>>> +    s->private = p;
>>>> +
>>>> +    return 0;
>>>> +}
>>>> +EXPORT_SYMBOL(seq_open_init);
>>>
>>> A global exported-to-modules interface should be documented, please.
>>> Especially when it has a void* argument.  seq_file.c is patchy - some
>>> of it is documented, some of it uses the read-programmers-mind
>>> approach.
>>
>> I have included documentation as the second patch. Would it have been
>> better to include them in a single patch? I didn't do that because
>> seq_file and Documentation have different maintainers. I'm still
>> learning the protocols here.
>
> Whoever merges the fs/ changes can (should) also merge the Documentation changes.

OK, if I resubmit (which seems quite likely), I'll merge them into a
single patch.

-- 
Rob Jones
Codethink Ltd
mailto:rob.jones@codethink.co.uk
tel:+44 161 236 5575

  reply	other threads:[~2014-09-25 17:39 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-09-24 11:15 [PATCH RESUBMIT 0/2] fs/seq_file: Add seq_open_init() Rob Jones
2014-09-24 11:15 ` [PATCH RESUBMIT 1/2] fs/seq_file: Create new function seq_open_init() Rob Jones
2014-09-24 21:39   ` Andrew Morton
2014-09-25  9:10     ` Rob Jones
2014-09-25 14:49       ` Randy Dunlap
2014-09-25 17:39         ` Rob Jones [this message]
2014-09-25 17:50       ` Andrew Morton
2014-09-25 17:54         ` Rob Jones
2014-09-25 18:07           ` Andrew Morton
2014-09-24 11:15 ` [PATCH RESUBMIT 2/2] Documentation/filesystem/seq_file: document seq_open_init() Rob Jones
2014-09-24 18:06 ` [PATCH RESUBMIT 0/2] fs/seq_file: Add seq_open_init() Kees Cook
2014-09-25  8:57   ` Rob Jones
2014-09-25 16:09     ` Kees Cook
2014-09-25 17:36       ` Rob Jones
2014-09-25 17:40         ` Kees Cook

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=54245351.1010701@codethink.co.uk \
    --to=rob.jones@codethink.co.uk \
    --cc=akpm@linux-foundation.org \
    --cc=keescook@chromium.org \
    --cc=linux-doc@vger.kernel.org \
    --cc=linux-fsdevel@vger.kernel.org \
    --cc=linux-kernel@codethink.co.uk \
    --cc=linux-kernel@vger.kernel.org \
    --cc=penguin-kernel@I-love.SAKURA.ne.jp \
    --cc=rdunlap@infradead.org \
    --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.