Wireshark-dev: Re: [Wireshark-dev] Val_to_str as a macro
From: Evan Huus <eapache@xxxxxxxxx>
Date: Wed, 11 Dec 2013 18:11:30 -0500
On Wed, Dec 11, 2013 at 5:40 PM, Jakub Zawadzki
<darkjames-ws@xxxxxxxxxxxx> wrote:
> Hi,
>
> On Wed, Dec 11, 2013 at 05:22:31PM -0500, Evan Huus wrote:
>> I've been exploring a few options for the val_to_str function and
>> wmem. One of the tangential things I've been trying to achieve is to
>> make it into a macro so that the compiler will catch format-string
>> mismatches (which have been a source of bugs in the past). This is
>> easy to do inefficiently:
>>
>> #define val_to_str(VAL, VS, POOL, FMT) (try_val_to_str(VAL, VS) ?
>> try_val_to_str(VAL, VS) : wmem_strdup_printf(POOL, FMT, VAL))
>>
>> but I've been unable to find a nice way to avoid the second call to
>> try_val_to_str(). I can wrap it in a block and use a temporary
>> variable, but then it can't return a value and no longer behaves like
>> the original function.
>
> We could use GCC extension: http://gcc.gnu.org/onlinedocs/gcc/Statement-Exprs.html
> (MSVC can't check format types anyway).
>
>> Can anybody else think of a nice way to do this, or is it just one of
>> those things that's impossible in C?
>
> Or more portable: I'd try moving val_to_str() as inline function to value_string.h
>
> and later just to test with something like: val_to_str(0, NULL, "%s");

Making the function inline is not enough to trigger the compiler
warning in my tests.

Another bad option: we could make it a more generic printf-style
function and pull the value out of the va_list, something like:

char* val_to_str(value_string *vs, char *fmt, ...) {
    va_list ap;
    int val;

    va_start(ap, fmt);
    val = va_arg(ap, int);
    va_end(ap);
    ...
}

This would correctly validate the format arguments, but unfortunately
it would then allow callers to not pass in any value at all, which is
just as bad. Maybe if we called that function _val_to_str and then
defined a wrapper macro with fixed arguments?

Actually, I think the generic-format-args function with a wrapper
macro might be the best option. Thoughts?

Evan