Re: Exporting functions from vsprintf.c for Rust

Re: Exporting functions from vsprintf.c for Rust

[Petr Mladek]

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>

Re: Exporting functions from vsprintf.c for Rust

[Remo Senekowitsch]

On Fri Apr 11, 2025 at 11:42 AM CEST, Petr Mladek wrote:
>
> 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);
> }

That makes sense. I tried your suggestion and it works, thank you!

Best regards,
Remo

Re: Exporting functions from vsprintf.c for Rust

[Andy Shevchenko]

On Fri, Apr 11, 2025 at 03:43:12PM +0200, Rasmus Villemoes wrote:
> On Fri, Apr 11, 2025, 13:36 Remo Senekowitsch <remo@buenzli.dev> wrote:
> > On Fri Apr 11, 2025 at 11:42 AM CEST, Petr Mladek wrote:

...

> > > 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);
> > > }
> >
> > That makes sense. I tried your suggestion and it works, thank you!
> >
> >
> But is a wrapper even needed for this? Can't the appropriate sprintf
> variant just be called from Rust with that %pfwf format string?

+1 here. But if really really need that wrapper, it should go to property.h.

-- 
With Best Regards,
Andy Shevchenko

Re: Exporting functions from vsprintf.c for Rust

[Remo Senekowitsch]

On Fri Apr 11, 2025 at 3:43 PM CEST, Rasmus Villemoes wrote:
> On Fri, Apr 11, 2025, 13:36 Remo Senekowitsch <remo@buenzli.dev> wrote:
>
>> On Fri Apr 11, 2025 at 11:42 AM CEST, Petr Mladek wrote:
>> >
>> > 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);
>> > }
>>
>> That makes sense. I tried your suggestion and it works, thank you!
>>
>>
> But is a wrapper even needed for this? Can't the appropriate sprintf
> variant just be called from Rust with that %pfwf format string?

Yeah, that works. Seems like the simplest solution - no C changes needed.
Thanks!

Remo