Re: Exporting functions from vsprintf.c for Rust

[Petr Mladek <pmladek@suse.com>]

Adding few more people and lkml into Cc.

On Thu 2025-04-10 17:10:57, Remo Senekowitsch wrote:
> Hi
> 
> I need to print the full path of a fwnode in Rust. One approach is to
> export it, as shown below. Is this acceptable to you in principle?
> 
> There are also some intermediary solutions like not providing a header
> and or prefixing `__`.
> 
> Thanks,
> Remo
> 
> ---
> diff --git a/include/linux/vsprintf.h b/include/linux/vsprintf.h
> new file mode 100644
> index 000000000..b37b11868
> --- /dev/null
> +++ b/include/linux/vsprintf.h

Just for record. Steven suggested to use include/linux/sprintf.h
instead because it was already used for another vsprintf APIs.

> @@ -0,0 +1,4 @@
> +// SPDX-License-Identifier: GPL-2.0
> +
> +char *fwnode_full_name_string(struct fwnode_handle *fwnode, char *buf,
> +			      char *end);

Honestly, I do not have a good feeling about exporting the internal
vsprintf() functions. They have a very specific semantic.

Especially, they return pointer to the next-to-write character.
And it might be even beyond the given *end pointer. It is because, for
example, vsnprintf() returns the number of characters which would
have been written to the buffer when it was big enough.

Instead, I suggest to create a wrapper which would have a sane
semantic and call scnprintf() internally. Something like:

int fwnode_full_name_to_string(char *buf, size_t size,
			       struct fwnode_handle *fwnode)
{
	return scnprintf(buf, size, "%pfwf", fwnode);
}

I am just not sure where to put it. It might be vsprintf.c.
But I think that a better place would be drivers/base/property.c.
For example, I see a "similar" fwnode_property_match_string()
already there.

Best Regards,
Petr


> diff --git a/lib/vsprintf.c b/lib/vsprintf.c
> index 56fe96319..3b4d0065a 100644
> --- a/lib/vsprintf.c
> +++ b/lib/vsprintf.c
> @@ -43,6 +43,7 @@
>  #include <linux/compiler.h>
>  #include <linux/property.h>
>  #include <linux/notifier.h>
> +#include <linux/vsprintf.h>
>  #ifdef CONFIG_BLOCK
>  #include <linux/blkdev.h>
>  #endif
> @@ -2103,7 +2104,7 @@ char *flags_string(char *buf, char *end, void *flags_ptr,
>  	return format_flags(buf, end, flags, names);
>  }
>  
> -static noinline_for_stack
> +noinline_for_stack
>  char *fwnode_full_name_string(struct fwnode_handle *fwnode, char *buf,
>  			      char *end)
>  {
> diff --git a/rust/bindings/bindings_helper.h b/rust/bindings/bindings_helper.h
> index 4bd02abd2..24a565ffd 100644
> --- a/rust/bindings/bindings_helper.h
> +++ b/rust/bindings/bindings_helper.h
> @@ -38,6 +38,7 @@
>  #include <linux/security.h>
>  #include <linux/slab.h>
>  #include <linux/tracepoint.h>
> +#include <linux/vsprintf.h>
>  #include <linux/wait.h>
>  #include <linux/workqueue.h>
>  #include <trace/events/rust_sample.h>