On 2018-04-10 21:05, Jeff Mahoney wrote:
> On 4/10/18 2:13 PM, Rasmus Villemoes wrote:
>> On 2018-04-10 16:25, Jan Kara wrote:
>>>
>>> Frankly, I don't like the format string parsing duplication in your patch
>>> and I'm afraid it will be a maintenance issue. Can't we have a variant
>>> of vsnprintf() that would take a pointer to va_list (like you use in
>>> your patch for some functions) so that its contents is well defined after
>>> the function returns?
>>
>> The problem with that is that (to avoid code duplication) the new
>> function would then have to be the main printf implementation, and the
>> existing vsnprintf() would have to be modified to use it, so there'd at
>> the very least be an extra indirection for each va_arg for all existing
>> sprintf users.
>>
>> And for extra fun, this won't actually work:
>>
>> int vsnprintf(char *buf, size_t len, const char *fmt, va_list ap)
>> {
>> return new_vsnprintf(buf, len, fmt, &ap);
>> }
>>
>> because if va_list is really a typedef for "struct blabla[1]" as on
>> x86-64, the "ap" inside vsnprintf is actually a "struct blabla *", and
>> new_sprintf() doesn't take a "struct blabla **", it takes a "pointer to
>> array 1 of struct blabla" aka "struct blabla (*)[1]". So either
>> vsnprintf() would need to be pessimized with an unconditional va_copy to
>> a locally declared va_list, or one could make the whole thing
>> arch-dependent (because on x86-64 and similar, we wouldn't actually need
>> new_vsnprintf). However, I'm not sure the i386 and x86-64 cases
>> represent all possible vararg implementations.
>>
>> But if somebody does implement something like new_vsnprintf, at least
>> we'd get all the internal calling conventions for all the various
>> architectures documented once and for all...
>
> Wouldn't this imply that %pV doesn't work today? That's what Jan and I
> are talking about: adding a %pV variant that doesn't va_copy the va_list
> before consuming it.
We can probably agree that %pV _does_ work today, on all architectures -
dev_printk etc. certainly relies on it. So what you're saying is really
just "you're wrong, Rasmus". But I can't see what part of what you quote
you think is wrong. Also, see my direct reply to 4/4 where I explain why
a non-copying %pV alone wouldn't solve the problem anyway. One really
would need a vsprintf variant that is guaranteed to take the va_list by
reference, and implementing that sanely without tree-wide implications
and working on all architectures is not trivial.
Rasmus
--
To unsubscribe from this list: send the line "unsubscribe reiserfs-devel" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html
