Re: [PATCH] seq_file: Fix seq_putc() to be consistent with seq_puts()

[Steven Rostedt]

On Mon, 29 Sep 2014 18:23:34 +0200
Petr Mladek <pmladek@xxxxxxx> wrote:
 
> Do you prefer:
> 
>       + the extra struct member?

I hate the extra member.

>       + or always filling some valid data?

Not needed.

>       + or invalidating the buffer when the overflow is set?

Not needed.

> 
> > > 
> > >   + always return error when seq_overflow() would return overflow;
> > >     in fact, the full buffer means that the last write operation
> > >     was most likely incomplete
> > 
> > I think this is probably the best option.
> 
> > 
> > > 
> > >     Also we might rename it from "overflow" to "full" or "filled"
> > >     or "sealed" or "complete" that might describe the state more
> > >     precisely because there is not a real overflow in some situations
> > 
> > We could make overflow be m->count > m->size, which would mean that
> > m->count == m->size (full) isn't a problem. Although, I would have to
> > audit the kernel to see if anything just assumes that m->count is
> > always less than m->size.
> 
> I am not sure if it is a good idea. I think that having
> m->count > m->size is prone to buffer overflow errors in
> the buffer reading code. I vote for adding extra flag to
> the struct :-)

Reading the seq_file code, it is really an all or nothing approach. It
even does the saving of the m->count and on overflow, will reset it to
what it did originally.

If an overflow happens, the buffer count should either be reset to what
it was (throw away anything that was written), or discarded completely,
which it sometimes does.

I'm going to write up some patches that converts overflow to be count =
size + 1, and then updating everything to be an all or nothing approach.

I'll take Joe's patches to convert them to void as well, with the
addition of a seq_is_full() helper for those places that wont waste time
adding more if it's just going to be discarded.

But first, I'm off to my doctor's appointment followed by more PT.

-- Steve
--
To unsubscribe from this list: send the line "unsubscribe linux-kernel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Please read the FAQ at  http://www.tux.org/lkml/