From: Joe Perches <joe@perches.com>
To: Ryan Mallon <rmallon@gmail.com>
Cc: Alexander Viro <viro@zeniv.linux.org.uk>,
Kees Cook <keescook@chromium.org>,
linux-fsdevel@vger.kernel.org, linux-kernel@vger.kernel.org
Subject: Re: [PATCH -next 1/3] seq: Add a seq_overflow test.
Date: Wed, 11 Dec 2013 15:31:41 -0800 [thread overview]
Message-ID: <1386804701.2479.2.camel@joe-AO722> (raw)
In-Reply-To: <52A8F4E0.5080402@gmail.com>
On Thu, 2013-12-12 at 10:27 +1100, Ryan Mallon wrote:
> On 11/12/13 16:12, Joe Perches wrote:
> > seq_printf and seq_puts returns are often misused.
> >
> > Instead of checking the seq_printf or seq_puts return,
> > add a new seq_overflow function to test if a seq_file has
> > overflowed the available buffer space.
> >
> > This will eventually allow seq_printf and seq_puts to be
> > converted to have a void return instead of the int return
> > that is often assumed to have a size_t value instead of an
> > error/no-error value.
> >
> > Signed-off-by: Joe Perches <joe@perches.com>
> > ---
> > fs/seq_file.c | 15 ++++++++-------
> > include/linux/seq_file.h | 2 ++
> > 2 files changed, 10 insertions(+), 7 deletions(-)
> >
> > diff --git a/fs/seq_file.c b/fs/seq_file.c
> > index 1d641bb..aab0736 100644
> > --- a/fs/seq_file.c
> > +++ b/fs/seq_file.c
> > @@ -14,16 +14,17 @@
> > #include <asm/uaccess.h>
> > #include <asm/page.h>
> >
> > -
> > -/*
> > - * seq_files have a buffer which can may overflow. When this happens a larger
> > - * buffer is reallocated and all the data will be printed again.
> > - * The overflow state is true when m->count == m->size.
> > +/**
> > + * seq_overflow - test if a seq_file has overflowed the space available
> > + * @m: the seq_file handle
> > + *
> > + * Returns -1 when an overflow has occurred, 0 otherwise.
> > */
> > -static bool seq_overflow(struct seq_file *m)
> > +int seq_overflow(struct seq_file *m)
> > {
> > - return m->count == m->size;
> > + return m->count == m->size ? -1 : 0;
> > }
> > +EXPORT_SYMBOL(seq_overflow);
>
> What is the reasoning in making this return int instead of bool? Having
> it return int encourages people to do:
>
> return seq_overflow(s);
>
> When used in seq_file functions will return -EPERM (-1) to user-space,
> which is confusing. It should probably return bool and let the caller
> sort out the correct error to return.
It was intended to make the return from seq_printf the same.
Anyway, the patch series is out of date (see Al's comments).
I'll update it later unless you or someone else does it first.
next prev parent reply other threads:[~2013-12-11 23:31 UTC|newest]
Thread overview: 5+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-12-11 5:12 [PATCH -next 0/3] seq_printf/puts/putc: Start to convert to return void Joe Perches
2013-12-11 5:12 ` [PATCH -next 1/3] seq: Add a seq_overflow test Joe Perches
2013-12-11 23:27 ` Ryan Mallon
2013-12-11 23:31 ` Joe Perches [this message]
2013-12-11 5:20 ` [PATCH -next 0/3] seq_printf/puts/putc: Start to convert to return void David Miller
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=1386804701.2479.2.camel@joe-AO722 \
--to=joe@perches.com \
--cc=keescook@chromium.org \
--cc=linux-fsdevel@vger.kernel.org \
--cc=linux-kernel@vger.kernel.org \
--cc=rmallon@gmail.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 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).