On Tue 10-04-18 20:13:31, 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...
Well, you are correct it won't work as I originally suggested but it should
work like:
#define vsnprintf(buf, len, fmt, ap) new_vsnprintf((buf), (len), (fmt), &(ap))
That way there's practically zero overhead for current vsnprintf() users
and no type issues...
Honza
--
Jan Kara <jack@xxxxxxxx>
SUSE Labs, CR
--
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
