Re: [PATCH v2] rust: add safety comment in rust_fmt_argument

[Rasmus Villemoes <ravi@prevas.dk>]

On Sat, Nov 23 2024, Konstantin Andrikopoulos <kernel@mandragore.io> wrote:

> On Thursday, November 21st, 2024 at 10:42 AM, Rasmus Villemoes <ravi@prevas.dk> wrote:
>
>> Nit: The file implementing this is vsprintf.c, but the actual workhorse
>> is vsnprintf() (note the n); all other *printf() variants are wrappers
>> around that, and I'm pretty sure no code path called from Rust ends up
>> passing through sprintf() or vsprintf(). Yes, I see that the original
>> code said vsprintf, but please fix while here.
>>
>
> Thanks for the comments! Going through the code again, I see that
> rust_fmt_argument is directly called from the pointer function in vsprintf.c
>
> But pointer is not only called from vsnprintf. It is also called in
> vbin_printf and bstr_printf. Should this be reflected in rust_fmt_argument's
> documentation, or are these functions unreachable from rust code?

As for the latter, I think they are used by the tracing code, so
probably they are or will be reachable from rust (Alice?). 

> Perhaps the docs should just mention pointer()? Maybe the original comment
> used "vsprintf" to refer collectively to all these functions?

That's certainly possible, I didn't think about that possibility. If so,
my comments should be ignored :)

The occurrence of vsprintf that triggered me was

  `vsprintf` guarantees that `buf` and `end` will uphold their
  requirements.

because while this could perhaps be read as
vsprintf-the-translation-unit, vsprintf-the-function blindly assumes
that its caller has passed a large enough buffer and fakes a len
argument of INT_MAX to vsnprintf().

So I dunno. If the intention is to refer to vsprintf.c as a whole,
perhaps it would be better to actually include that .c extension when
referring to it, to avoid the ambiguity.

Rasmus