On 4/10/18 2:13 PM, Rasmus Villemoes wrote:
> On 2018-04-10 16:25, Jan Kara wrote:
>> On Mon 09-04-18 14:16:02, jeffm@xxxxxxxx wrote:
>>> From: Jeff Mahoney <jeffm@xxxxxxxx>
>>>
>>
>>> The result is a va_list that is consistent within reiserfs_snprintf
>>> and isn't passed by value anymore.
>>>
>>> This isn't an ideal solution, but I didn't feel that teaching pointer()
>>> about a %pV variant that didn't use va_copy would get much traction for
>>> a number of reasons.
>>
>> 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.
-Jeff
--
Jeff Mahoney
SUSE Labs
--
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
