On Fri, 2012-06-01 at 11:39 +0100, Steven Whitehouse wrote:
> I've just been taking a look at the seq_read() code,
[]

nice improvement.  trivial style comment:

> diff --git a/fs/seq_file.c b/fs/seq_file.c
[]
> @@ -169,6 +171,16 @@ ssize_t seq_read(struct file *file, char __user *buf, 
> size_t size, loff_t *ppos)
>        */
>       m->version = file->f_version;
>  
> +     if ((m->size < bsize) || !m->buf) {
> +             kfree(m->buf);
> +             m->size = 0;
> +             m->buf = kmalloc(m->size = bsize, GFP_KERNEL|__GFP_NOWARN);

You can remove the m->size = 0 line and
can you please add a space around the |?
It's a bit dense and too easy to read it
as a single word.
 
> +             if (!m->buf)
> +                     m->buf = kmalloc(m->size = PAGE_SIZE, GFP_KERNEL);
> +             if (!m->buf)
> +                     goto Enomem;

Mixed case label?

> @@ -277,6 +289,11 @@ Done:
>               m->read_pos += copied;
>       }
>       file->f_version = m->version;
> +     if (m->size > PAGE_SIZE) {
> +             kfree(m->buf);
> +             m->buf = NULL;
> +             m->size = 0;
> +     }
>       mutex_unlock(&m->lock);
>       return copied;
>  Enomem:

Oh, it wasn't you.  nevermind...

Reply via email to