Re: [PATCH 4/4] reiserfs: rework reiserfs_snprintf to not abuse va_list (as much)

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



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...

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



[Index of Archives]     [Linux File System Development]     [Linux BTRFS]     [Linux NFS]     [Linux Filesystems]     [Ext4 Filesystem]     [Kernel Newbies]     [Share Photos]     [Security]     [Netfilter]     [Bugtraq]     [Yosemite Forum]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Samba]     [Device Mapper]     [Linux Resources]

  Powered by Linux