2014-11-04 16:05:20

by Steven Rostedt

[permalink] [raw]
Subject: [RFC][PATCH 10/12 v3] seq-buf: Make seq_buf_bprintf() conditional on CONFIG_BINARY_PRINTF

From: "Steven Rostedt (Red Hat)" <[email protected]>

The function bstr_printf() from lib/vsprnintf.c is only available if
CONFIG_BINARY_PRINTF is defined. This is due to the only user currently
being the tracing infrastructure, which needs to select this config
when tracing is configured. Until there is another user of the binary
printf formats, this will continue to be the case.

Since seq_buf.c is now lives in lib/ and is compiled even without
tracing, it must encompass its use of bstr_printf() which is used
by seq_buf_printf(). This too is only used by the tracing infrastructure
and is still encapsulated by the CONFIG_BINARY_PRINTF.

Signed-off-by: Steven Rostedt <[email protected]>
---
include/linux/seq_buf.h | 7 +++++--
lib/seq_buf.c | 2 ++
2 files changed, 7 insertions(+), 2 deletions(-)

diff --git a/include/linux/seq_buf.h b/include/linux/seq_buf.h
index 10c17c61c7a4..b6291e2de99d 100644
--- a/include/linux/seq_buf.h
+++ b/include/linux/seq_buf.h
@@ -99,8 +99,6 @@ extern __printf(2, 3)
int seq_buf_printf(struct seq_buf *s, const char *fmt, ...);
extern __printf(2, 0)
int seq_buf_vprintf(struct seq_buf *s, const char *fmt, va_list args);
-extern int
-seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary);
extern int seq_buf_print_seq(struct seq_file *m, struct seq_buf *s);
extern int seq_buf_to_user(struct seq_buf *s, char __user *ubuf,
int cnt);
@@ -114,4 +112,9 @@ extern int seq_buf_path(struct seq_buf *s, const struct path *path, const char *
extern int seq_buf_bitmask(struct seq_buf *s, const unsigned long *maskp,
int nmaskbits);

+#ifdef CONFIG_BINARY_PRINTF
+extern int
+seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary);
+#endif
+
#endif /* _LINUX_SEQ_BUF_H */
diff --git a/lib/seq_buf.c b/lib/seq_buf.c
index d118d0d6c956..ad09f8e6f2a6 100644
--- a/lib/seq_buf.c
+++ b/lib/seq_buf.c
@@ -114,6 +114,7 @@ int seq_buf_bitmask(struct seq_buf *s, const unsigned long *maskp,
return -1;
}

+#ifdef CONFIG_BINARY_PRINTF
/**
* seq_buf_bprintf - Write the printf string from binary arguments
* @s: seq_buf descriptor
@@ -148,6 +149,7 @@ int seq_buf_bprintf(struct seq_buf *s, const char *fmt, const u32 *binary)
seq_buf_set_overflow(s);
return -1;
}
+#endif /* CONFIG_BINARY_PRINTF */

/**
* seq_buf_puts - sequence printing of simple string
--
2.1.1


2014-11-05 17:06:21

by Petr Mladek

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/12 v3] seq-buf: Make seq_buf_bprintf() conditional on CONFIG_BINARY_PRINTF

On Tue 2014-11-04 10:52:47, Steven Rostedt wrote:
> From: "Steven Rostedt (Red Hat)" <[email protected]>
>
> The function bstr_printf() from lib/vsprnintf.c is only available if
> CONFIG_BINARY_PRINTF is defined. This is due to the only user currently
> being the tracing infrastructure, which needs to select this config
> when tracing is configured. Until there is another user of the binary
> printf formats, this will continue to be the case.
>
> Since seq_buf.c is now lives in lib/ and is compiled even without
> tracing, it must encompass its use of bstr_printf() which is used
> by seq_buf_printf(). This too is only used by the tracing infrastructure
> and is still encapsulated by the CONFIG_BINARY_PRINTF.
>
> Signed-off-by: Steven Rostedt <[email protected]>

Please switch the order and do this change before moving to lib/.
IMHO, the current order would break bisecting when tracing is
disabled.

Otherwise, the change looks good in itself, so:

Reviewed-by: Petr Mladek <[email protected]>

Best Regards,
Petr

2014-11-05 20:34:14

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/12 v3] seq-buf: Make seq_buf_bprintf() conditional on CONFIG_BINARY_PRINTF

On Wed, 5 Nov 2014 18:06:05 +0100
Petr Mladek <[email protected]> wrote:

> On Tue 2014-11-04 10:52:47, Steven Rostedt wrote:
> > From: "Steven Rostedt (Red Hat)" <[email protected]>
> >
> > The function bstr_printf() from lib/vsprnintf.c is only available if
> > CONFIG_BINARY_PRINTF is defined. This is due to the only user currently
> > being the tracing infrastructure, which needs to select this config
> > when tracing is configured. Until there is another user of the binary
> > printf formats, this will continue to be the case.
> >
> > Since seq_buf.c is now lives in lib/ and is compiled even without
> > tracing, it must encompass its use of bstr_printf() which is used
> > by seq_buf_printf(). This too is only used by the tracing infrastructure
> > and is still encapsulated by the CONFIG_BINARY_PRINTF.
> >
> > Signed-off-by: Steven Rostedt <[email protected]>
>
> Please switch the order and do this change before moving to lib/.
> IMHO, the current order would break bisecting when tracing is
> disabled.

I agree. I was being lazy and when my test broke, I added the update to
test again. But to move this change to before the move, I need to
monkey with the patch file.

I'll move it before the move regardless.

>
> Otherwise, the change looks good in itself, so:
>
> Reviewed-by: Petr Mladek <[email protected]>

Thanks,

-- Steve

2014-11-05 20:42:11

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/12 v3] seq-buf: Make seq_buf_bprintf() conditional on CONFIG_BINARY_PRINTF

On Wed, 5 Nov 2014 15:33:55 -0500
Steven Rostedt <[email protected]> wrote:

> On Wed, 5 Nov 2014 18:06:05 +0100
> Petr Mladek <[email protected]> wrote:
>
> > On Tue 2014-11-04 10:52:47, Steven Rostedt wrote:
> > > From: "Steven Rostedt (Red Hat)" <[email protected]>
> > >
> > > The function bstr_printf() from lib/vsprnintf.c is only available if
> > > CONFIG_BINARY_PRINTF is defined. This is due to the only user currently
> > > being the tracing infrastructure, which needs to select this config
> > > when tracing is configured. Until there is another user of the binary
> > > printf formats, this will continue to be the case.
> > >
> > > Since seq_buf.c is now lives in lib/ and is compiled even without
> > > tracing, it must encompass its use of bstr_printf() which is used
> > > by seq_buf_printf(). This too is only used by the tracing infrastructure
> > > and is still encapsulated by the CONFIG_BINARY_PRINTF.
> > >
> > > Signed-off-by: Steven Rostedt <[email protected]>
> >
> > Please switch the order and do this change before moving to lib/.
> > IMHO, the current order would break bisecting when tracing is
> > disabled.
>
> I agree. I was being lazy and when my test broke, I added the update to
> test again. But to move this change to before the move, I need to
> monkey with the patch file.
>
> I'll move it before the move regardless.
>

OK, 'git rebase' is SUPER AWESOME!!!

I just did a git rebase, moved this change to before the file move, and
git somehow knew that the change was for the old file, and updated it
without any modification from me. The rebase simply succeeded!

I checked, the change still changed seq_buf.c, but in the
old kernel/trace directory.

-- Steve

2014-11-06 14:39:46

by Petr Mladek

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/12 v3] seq-buf: Make seq_buf_bprintf() conditional on CONFIG_BINARY_PRINTF

On Wed 2014-11-05 15:42:03, Steven Rostedt wrote:
> On Wed, 5 Nov 2014 15:33:55 -0500
> Steven Rostedt <[email protected]> wrote:
>
> > On Wed, 5 Nov 2014 18:06:05 +0100
> > Petr Mladek <[email protected]> wrote:
> >
> > > On Tue 2014-11-04 10:52:47, Steven Rostedt wrote:
> > > > From: "Steven Rostedt (Red Hat)" <[email protected]>
> > > >
> > > > The function bstr_printf() from lib/vsprnintf.c is only available if
> > > > CONFIG_BINARY_PRINTF is defined. This is due to the only user currently
> > > > being the tracing infrastructure, which needs to select this config
> > > > when tracing is configured. Until there is another user of the binary
> > > > printf formats, this will continue to be the case.
> > > >
> > > > Since seq_buf.c is now lives in lib/ and is compiled even without
> > > > tracing, it must encompass its use of bstr_printf() which is used
> > > > by seq_buf_printf(). This too is only used by the tracing infrastructure
> > > > and is still encapsulated by the CONFIG_BINARY_PRINTF.
> > > >
> > > > Signed-off-by: Steven Rostedt <[email protected]>
> > >
> > > Please switch the order and do this change before moving to lib/.
> > > IMHO, the current order would break bisecting when tracing is
> > > disabled.
> >
> > I agree. I was being lazy and when my test broke, I added the update to
> > test again. But to move this change to before the move, I need to
> > monkey with the patch file.
> >
> > I'll move it before the move regardless.
> >
>
> OK, 'git rebase' is SUPER AWESOME!!!
>
> I just did a git rebase, moved this change to before the file move, and
> git somehow knew that the change was for the old file, and updated it
> without any modification from me. The rebase simply succeeded!
>
> I checked, the change still changed seq_buf.c, but in the
> old kernel/trace directory.

That is really awesome! Dreams become reality.

Best Regards,
Petr

2014-11-07 20:36:10

by Junio C Hamano

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/12 v3] seq-buf: Make seq_buf_bprintf() conditional on CONFIG_BINARY_PRINTF

Petr Mladek <[email protected]> writes:

> On Wed 2014-11-05 15:42:03, Steven Rostedt wrote:
> ...
>>
>> OK, 'git rebase' is SUPER AWESOME!!!
>>
>> I just did a git rebase, moved this change to before the file move, and
>> git somehow knew that the change was for the old file, and updated it
>> without any modification from me. The rebase simply succeeded!
>>
>> I checked, the change still changed seq_buf.c, but in the
>> old kernel/trace directory.
>
> That is really awesome! Dreams become reality.

It is rare to hear somebody talks about Git without having "sucks"
in the same sentence ;-)

2014-11-07 21:50:06

by Steven Rostedt

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/12 v3] seq-buf: Make seq_buf_bprintf() conditional on CONFIG_BINARY_PRINTF

On Fri, 07 Nov 2014 12:36:05 -0800
Junio C Hamano <[email protected]> wrote:

> Petr Mladek <[email protected]> writes:
>
> > On Wed 2014-11-05 15:42:03, Steven Rostedt wrote:
> > ...
> >>
> >> OK, 'git rebase' is SUPER AWESOME!!!
> >>
> >> I just did a git rebase, moved this change to before the file move, and
> >> git somehow knew that the change was for the old file, and updated it
> >> without any modification from me. The rebase simply succeeded!
> >>
> >> I checked, the change still changed seq_buf.c, but in the
> >> old kernel/trace directory.
> >
> > That is really awesome! Dreams become reality.
>
> It is rare to hear somebody talks about Git without having "sucks"
> in the same sentence ;-)

Yes, that's why I Cc'd you. I figured you would want to hear it.

-- Steve

2014-11-10 18:43:29

by Petr Mladek

[permalink] [raw]
Subject: Re: [RFC][PATCH 10/12 v3] seq-buf: Make seq_buf_bprintf() conditional on CONFIG_BINARY_PRINTF

On Fri 2014-11-07 12:36:05, Junio C Hamano wrote:
> Petr Mladek <[email protected]> writes:
>
> > On Wed 2014-11-05 15:42:03, Steven Rostedt wrote:
> > ...
> >>
> >> OK, 'git rebase' is SUPER AWESOME!!!
> >>
> >> I just did a git rebase, moved this change to before the file move, and
> >> git somehow knew that the change was for the old file, and updated it
> >> without any modification from me. The rebase simply succeeded!
> >>
> >> I checked, the change still changed seq_buf.c, but in the
> >> old kernel/trace directory.
> >
> > That is really awesome! Dreams become reality.
>
> It is rare to hear somebody talks about Git without having "sucks"
> in the same sentence ;-)

This might be an effect of bugzilla and other bugreport tools. One
gets the feeling that there are only people strugling with the software.
In fact, there is usually much bigger group of happy users. They
just keep the happines inside until something goes wrong :-)

I am happy git user (most of the time :-)

Best Regards,
Petr