From: "Dr. David Alan Gilbert" <linux@treblig.org>

bprintf is unused.  Remove it.

It was added in
commit 4370aa4aa753 ("vsprintf: add binary printf")

but as far as I can see was never used, unlike the other
two functions in that patch.

Signed-off-by: Dr. David Alan Gilbert <linux@treblig.org>
---
 include/linux/string.h |  1 -
 lib/vsprintf.c         | 23 -----------------------
 2 files changed, 24 deletions(-)

diff --git a/include/linux/string.h b/include/linux/string.h
index 0dd27afcfaf7..493ac4862c77 100644
--- a/include/linux/string.h
+++ b/include/linux/string.h
@@ -335,7 +335,6 @@ int __sysfs_match_string(const char * const *array, size_t n, const char *s);
 #ifdef CONFIG_BINARY_PRINTF
 int vbin_printf(u32 *bin_buf, size_t size, const char *fmt, va_list args);
 int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf);
-int bprintf(u32 *bin_buf, size_t size, const char *fmt, ...) __printf(3, 4);
 #endif
 
 extern ssize_t memory_read_from_buffer(void *to, size_t count, loff_t *ppos,
diff --git a/lib/vsprintf.c b/lib/vsprintf.c
index 09f022ba1c05..d072cd59eb6a 100644
--- a/lib/vsprintf.c
+++ b/lib/vsprintf.c
@@ -3383,29 +3383,6 @@ int bstr_printf(char *buf, size_t size, const char *fmt, const u32 *bin_buf)
 }
 EXPORT_SYMBOL_GPL(bstr_printf);
 
-/**
- * bprintf - Parse a format string and place args' binary value in a buffer
- * @bin_buf: The buffer to place args' binary value
- * @size: The size of the buffer(by words(32bits), not characters)
- * @fmt: The format string to use
- * @...: Arguments for the format string
- *
- * The function returns the number of words(u32) written
- * into @bin_buf.
- */
-int bprintf(u32 *bin_buf, size_t size, const char *fmt, ...)
-{
-	va_list args;
-	int ret;
-
-	va_start(args, fmt);
-	ret = vbin_printf(bin_buf, size, fmt, args);
-	va_end(args);
-
-	return ret;
-}
-EXPORT_SYMBOL_GPL(bprintf);
-
 #endif /* CONFIG_BINARY_PRINTF */
 
 /**
-- 
2.46.2

On Wed, Oct 02, 2024 at 02:21:25AM +0100, linux@treblig.org wrote:
> From: "Dr. David Alan Gilbert" <linux@treblig.org>
> 
> bprintf is unused.  Remove it.

bprintf()

> It was added in
> commit 4370aa4aa753 ("vsprintf: add binary printf")
> 
> but as far as I can see was never used, unlike the other
> two functions in that patch.

Please, rewrap these lines to use more room on each line.

...

I am not familiar with tricks in BPF or ftrace code where this actually might
be implicitly called via a macro, but brief grep gives nothing that might point
to that. Hence, with the amended commit message

Reviewed-by: Andy Shevchenko <andy@kernel.org>

P.S. I hope we may rely in CIs to report issues soon, if any.

-- 
With Best Regards,
Andy Shevchenko

* Andy Shevchenko (andy@kernel.org) wrote:
> On Wed, Oct 02, 2024 at 02:21:25AM +0100, linux@treblig.org wrote:
> > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> > 
> > bprintf is unused.  Remove it.
> 
> bprintf()

OK.

> > It was added in
> > commit 4370aa4aa753 ("vsprintf: add binary printf")
> > 
> > but as far as I can see was never used, unlike the other
> > two functions in that patch.
> 
> Please, rewrap these lines to use more room on each line.

Something like this? :

bprintf() is unused.  Remove it.  It was added in

commit 4370aa4aa753 ("vsprintf: add binary printf")

but as far as I can see was never used, unlike the other two functions in
that patch.


> ...
> 
> I am not familiar with tricks in BPF or ftrace code where this actually might
> be implicitly called via a macro, but brief grep gives nothing that might point
> to that.

I've got an all-yes build (well, most after I took out broken stuff) booting
with it, and it has CONFIG_BINARY_PRINTF=y and CONFIG_FTRACE=y .

trace_seq.c uses seq_buf_bprintf which uses bstr_printf rather than the plain
bprintf() that I've deleted.
Not sure where to dig in BPF, but I've had a fairly good grep around.

> Hence, with the amended commit message
> 
> Reviewed-by: Andy Shevchenko <andy@kernel.org>

Thanks!

> P.S. I hope we may rely in CIs to report issues soon, if any.

Sure

Dave

> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

On Wed, 2 Oct 2024 14:31:19 +0000
"Dr. David Alan Gilbert" <linux@treblig.org> wrote:

> > I am not familiar with tricks in BPF or ftrace code where this actually might
> > be implicitly called via a macro, but brief grep gives nothing that might point
> > to that.  
> 
> I've got an all-yes build (well, most after I took out broken stuff) booting
> with it, and it has CONFIG_BINARY_PRINTF=y and CONFIG_FTRACE=y .
> 
> trace_seq.c uses seq_buf_bprintf which uses bstr_printf rather than the plain
> bprintf() that I've deleted.
> Not sure where to dig in BPF, but I've had a fairly good grep around.

I believe it is safe to delete. It looks like bprintf() was added for
completeness, where as everything is just using the vbin_printf() directly.
bprintf() is nothing more than a wrapper around it in case someone wanted
to use binary prints directly. But I'm not sure there's a good use case for
it, as all users would likely need to add some code around it for
processing (like trace.c does).

Send a v2 and I could take it for v6.13.

-- Steve

* Steven Rostedt (rostedt@goodmis.org) wrote:
> On Wed, 2 Oct 2024 14:31:19 +0000
> "Dr. David Alan Gilbert" <linux@treblig.org> wrote:
> 
> > > I am not familiar with tricks in BPF or ftrace code where this actually might
> > > be implicitly called via a macro, but brief grep gives nothing that might point
> > > to that.  
> > 
> > I've got an all-yes build (well, most after I took out broken stuff) booting
> > with it, and it has CONFIG_BINARY_PRINTF=y and CONFIG_FTRACE=y .
> > 
> > trace_seq.c uses seq_buf_bprintf which uses bstr_printf rather than the plain
> > bprintf() that I've deleted.
> > Not sure where to dig in BPF, but I've had a fairly good grep around.
> 
> I believe it is safe to delete. It looks like bprintf() was added for
> completeness, where as everything is just using the vbin_printf() directly.
> bprintf() is nothing more than a wrapper around it in case someone wanted
> to use binary prints directly. But I'm not sure there's a good use case for
> it, as all users would likely need to add some code around it for
> processing (like trace.c does).
> 
> Send a v2 and I could take it for v6.13.

Thanks,

Just posted, message-id 20241002173147.210107-1-linux@treblig.org

Dave

> -- Steve
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/

On Wed, Oct 02, 2024 at 02:31:19PM +0000, Dr. David Alan Gilbert wrote:
> * Andy Shevchenko (andy@kernel.org) wrote:
> > On Wed, Oct 02, 2024 at 02:21:25AM +0100, linux@treblig.org wrote:
> > > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> > > 
> > > bprintf is unused.  Remove it.
> > 
> > bprintf()
> 
> OK.
> 
> > > It was added in
> > > commit 4370aa4aa753 ("vsprintf: add binary printf")
> > > 
> > > but as far as I can see was never used, unlike the other
> > > two functions in that patch.
> > 
> > Please, rewrap these lines to use more room on each line.
> 
> Something like this? :

> bprintf() is unused.  Remove it.  It was added in
> 
> commit 4370aa4aa753 ("vsprintf: add binary printf")
> 
> but as far as I can see was never used, unlike the other two functions in
> that patch.

You have two blank lines in the middle, do you really need them?
I would put it like this

bprintf() is unused. Remove it. It was added in the commit 4370aa4aa753
("vsprintf: add binary printf") but as far as I can see was never used,
unlike the other two functions in that patch.

-- 
With Best Regards,
Andy Shevchenko

* Andy Shevchenko (andy@kernel.org) wrote:
> On Wed, Oct 02, 2024 at 02:31:19PM +0000, Dr. David Alan Gilbert wrote:
> > * Andy Shevchenko (andy@kernel.org) wrote:
> > > On Wed, Oct 02, 2024 at 02:21:25AM +0100, linux@treblig.org wrote:
> > > > From: "Dr. David Alan Gilbert" <linux@treblig.org>
> > > > 
> > > > bprintf is unused.  Remove it.
> > > 
> > > bprintf()
> > 
> > OK.
> > 
> > > > It was added in
> > > > commit 4370aa4aa753 ("vsprintf: add binary printf")
> > > > 
> > > > but as far as I can see was never used, unlike the other
> > > > two functions in that patch.
> > > 
> > > Please, rewrap these lines to use more room on each line.
> > 
> > Something like this? :
> 
> > bprintf() is unused.  Remove it.  It was added in
> > 
> > commit 4370aa4aa753 ("vsprintf: add binary printf")
> > 
> > but as far as I can see was never used, unlike the other two functions in
> > that patch.
> 
> You have two blank lines in the middle, do you really need them?

No, I was just trying to keep the commit reference separate, I'm never
quite sure what checkpatch gets upset by.

> I would put it like this
> 
> bprintf() is unused. Remove it. It was added in the commit 4370aa4aa753
> ("vsprintf: add binary printf") but as far as I can see was never used,
> unlike the other two functions in that patch.

Sure, I can send a v2 like that.

Dave

> -- 
> With Best Regards,
> Andy Shevchenko
> 
> 
-- 
 -----Open up your eyes, open up your mind, open up your code -------   
/ Dr. David Alan Gilbert    |       Running GNU/Linux       | Happy  \ 
\        dave @ treblig.org |                               | In Hex /
 \ _________________________|_____ http://www.treblig.org   |_______/