From: "Steven Rostedt (Red Hat)" <[email protected]>
Add a helper function seq_buf_used() that replaces the SEQ_BUF_USED()
private macro to let callers have a method to know how much of the
seq_buf was written to.
Link: http://lkml.kernel.org/r/[email protected]
Link: http://lkml.kernel.org/r/[email protected]
Signed-off-by: Steven Rostedt <[email protected]>
---
include/linux/seq_buf.h | 6 ++++++
kernel/trace/seq_buf.c | 5 +----
2 files changed, 7 insertions(+), 4 deletions(-)
diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index 5d91262433e2..93718e570d4c 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -64,6 +64,12 @@ seq_buf_buffer_left(struct seq_buf *s)
return (s->size - 1) - s->len;
}
+/* How much buffer was written? */
+static inline unsigned int seq_buf_used(struct seq_buf *s)
+{
+ return min(s->len, s->size);
+}
+
extern __printf(2, 3)
int seq_buf_printf(struct seq_buf *s, const char *fmt, ...);
extern __printf(2, 0)
diff --git a/kernel/trace/seq_buf.c b/kernel/trace/seq_buf.c
index 7dac34d1235b..9ec5305d9da7 100644
--- a/kernel/trace/seq_buf.c
+++ b/kernel/trace/seq_buf.c
@@ -16,9 +16,6 @@
#include <linux/seq_file.h>
#include <linux/seq_buf.h>
-/* How much buffer is written? */
-#define SEQ_BUF_USED(s) min((s)->len, (s)->size - 1)
-
/**
* seq_buf_print_seq - move the contents of seq_buf into a seq_file
* @m: the seq_file descriptor that is the destination
@@ -28,7 +25,7 @@
*/
int seq_buf_print_seq(struct seq_file *m, struct seq_buf *s)
{
- unsigned int len = SEQ_BUF_USED(s);
+ unsigned int len = seq_buf_used(s);
return seq_write(m, s->buffer, len);
}
--
2.1.1
On Fri 2014-11-14 23:59:05, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" <[email protected]>
>
> Add a helper function seq_buf_used() that replaces the SEQ_BUF_USED()
> private macro to let callers have a method to know how much of the
> seq_buf was written to.
>
> Link: http://lkml.kernel.org/r/[email protected]
> Link: http://lkml.kernel.org/r/[email protected]
>
> Signed-off-by: Steven Rostedt <[email protected]>
> ---
> include/linux/seq_buf.h | 6 ++++++
> kernel/trace/seq_buf.c | 5 +----
> 2 files changed, 7 insertions(+), 4 deletions(-)
>
> diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
> index 5d91262433e2..93718e570d4c 100644
> --- a/include/linux/seq_buf.h
> +++ b/include/linux/seq_buf.h
> @@ -64,6 +64,12 @@ seq_buf_buffer_left(struct seq_buf *s)
> return (s->size - 1) - s->len;
> }
>
> +/* How much buffer was written? */
> +static inline unsigned int seq_buf_used(struct seq_buf *s)
> +{
> + return min(s->len, s->size);
To be precise, it should do
return min(s->len, s->size - 1);
at this stage and switch to the above code in ("[PATCH 21/26 v5] tracing: Have
seq_buf use full buffer"). Well, it does not cause any big harm. Feel
free to add:
Reviewed-by: Petr Mladek <[email protected]>
for this patch or for the updated one.
Best Regards,
Petr
> +}
> +
> extern __printf(2, 3)
> int seq_buf_printf(struct seq_buf *s, const char *fmt, ...);
> extern __printf(2, 0)
> diff --git a/kernel/trace/seq_buf.c b/kernel/trace/seq_buf.c
> index 7dac34d1235b..9ec5305d9da7 100644
> --- a/kernel/trace/seq_buf.c
> +++ b/kernel/trace/seq_buf.c
> @@ -16,9 +16,6 @@
> #include <linux/seq_file.h>
> #include <linux/seq_buf.h>
>
> -/* How much buffer is written? */
> -#define SEQ_BUF_USED(s) min((s)->len, (s)->size - 1)
> -
> /**
> * seq_buf_print_seq - move the contents of seq_buf into a seq_file
> * @m: the seq_file descriptor that is the destination
> @@ -28,7 +25,7 @@
> */
> int seq_buf_print_seq(struct seq_file *m, struct seq_buf *s)
> {
> - unsigned int len = SEQ_BUF_USED(s);
> + unsigned int len = seq_buf_used(s);
>
> return seq_write(m, s->buffer, len);
> }
> --
> 2.1.1
>
>
On Tue, 18 Nov 2014 16:02:04 +0100
Petr Mladek <[email protected]> wrote:
> On Fri 2014-11-14 23:59:05, Steven Rostedt wrote:
> > From: "Steven Rostedt (Red Hat)" <[email protected]>
> >
> > Add a helper function seq_buf_used() that replaces the SEQ_BUF_USED()
> > private macro to let callers have a method to know how much of the
> > seq_buf was written to.
> >
> > Link: http://lkml.kernel.org/r/[email protected]
> > Link: http://lkml.kernel.org/r/[email protected]
> >
> > Signed-off-by: Steven Rostedt <[email protected]>
> > ---
> > include/linux/seq_buf.h | 6 ++++++
> > kernel/trace/seq_buf.c | 5 +----
> > 2 files changed, 7 insertions(+), 4 deletions(-)
> >
> > diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
> > index 5d91262433e2..93718e570d4c 100644
> > --- a/include/linux/seq_buf.h
> > +++ b/include/linux/seq_buf.h
> > @@ -64,6 +64,12 @@ seq_buf_buffer_left(struct seq_buf *s)
> > return (s->size - 1) - s->len;
> > }
> >
> > +/* How much buffer was written? */
> > +static inline unsigned int seq_buf_used(struct seq_buf *s)
> > +{
> > + return min(s->len, s->size);
>
> To be precise, it should do
>
> return min(s->len, s->size - 1);
>
> at this stage and switch to the above code in ("[PATCH 21/26 v5] tracing: Have
> seq_buf use full buffer"). Well, it does not cause any big harm. Feel
> free to add:
I actually thought about it, but realized that this only replaces the
original use of s->len, and we are only doing this to avoid buffer
overflows. If you get garbage, then so be it, as the original code
would give garbage too. Remember, some of the functions did have real
content in that last byte, even though it was considered "overflowed".
-- Steve
>
> Reviewed-by: Petr Mladek <[email protected]>
>
> for this patch or for the updated one.
>
On Wed 2014-11-19 10:49:56, Steven Rostedt wrote:
> On Tue, 18 Nov 2014 16:02:04 +0100
> Petr Mladek <[email protected]> wrote:
>
> > On Fri 2014-11-14 23:59:05, Steven Rostedt wrote:
> > > From: "Steven Rostedt (Red Hat)" <[email protected]>
> > >
> > > Add a helper function seq_buf_used() that replaces the SEQ_BUF_USED()
> > > private macro to let callers have a method to know how much of the
> > > seq_buf was written to.
> > >
> > > Link: http://lkml.kernel.org/r/[email protected]
> > > Link: http://lkml.kernel.org/r/[email protected]
> > >
> > > Signed-off-by: Steven Rostedt <[email protected]>
> > > ---
> > > include/linux/seq_buf.h | 6 ++++++
> > > kernel/trace/seq_buf.c | 5 +----
> > > 2 files changed, 7 insertions(+), 4 deletions(-)
> > >
> > > diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
> > > index 5d91262433e2..93718e570d4c 100644
> > > --- a/include/linux/seq_buf.h
> > > +++ b/include/linux/seq_buf.h
> > > @@ -64,6 +64,12 @@ seq_buf_buffer_left(struct seq_buf *s)
> > > return (s->size - 1) - s->len;
> > > }
> > >
> > > +/* How much buffer was written? */
> > > +static inline unsigned int seq_buf_used(struct seq_buf *s)
> > > +{
> > > + return min(s->len, s->size);
> >
> > To be precise, it should do
> >
> > return min(s->len, s->size - 1);
> >
> > at this stage and switch to the above code in ("[PATCH 21/26 v5] tracing: Have
> > seq_buf use full buffer"). Well, it does not cause any big harm. Feel
> > free to add:
>
> I actually thought about it, but realized that this only replaces the
> original use of s->len, and we are only doing this to avoid buffer
> overflows. If you get garbage, then so be it, as the original code
> would give garbage too. Remember, some of the functions did have real
> content in that last byte, even though it was considered "overflowed".
I thought that it was based on ftrace_seq code that had
#define TRACE_SEQ_BUF_USED(s) min((s)->len, (unsigned int)(PAGE_SIZE -
1))
But you are right that the callers used s->len because the macro was
local in trace_seq.c.
Well, I think that it does not matter much. As you said, there might be
a garbage a the end of the buffer. Callers used "len" directly.
Also it will be min(s->len, s->size); after the commit "Have seq_buf
use full buffer".
Let's leave it as is.
Best Regards,
Petr