Re: [PATCH] fix up external acl type dumping

On 06/15/2012 06:16 AM, Henrik Nordström wrote:
> tor 2012-06-14 klockan 10:23 -0600 skrev Alex Rousskov:
>
>>> +#define DUMP_EXT_ACL_TYPE_FMT(a, fmt, ...) \
>>> + case _external_acl_format::EXT_ACL_##a: \
>>> + storeAppendPrintf(sentry, fmt, ##__VA_ARGS__); \
>>> + break
>>
>> I do not see Squid using __VA_ARGS__ currently. Are you sure it is
>> portable, especially in the ## context? If you have not tested this
>> outside Linux, are you OK with us pulling out this patch if it breaks
>> the build?
>
> Checking.. seems all platform we care about should support it by now.
>
> http://en.wikipedia.org/wiki/Variadic_macro
>
> But there is some variance on how an empty list of variadic arguments is
> handled. Some require at least one argument to avoid syntax errors in
> resulting code. (the , before ##__VA_ARGS__ always emitted)

And since the patch does not always supply that argument, the
probability that we will have to fix this after commit is high.

IMHO, ##__VA_ARGS__ is not worth the trouble in this particular case.
However, even if you disagree, please use at least one argument (empty
string with a corresponding %s if needed to prevent compiler warnings?).

Thank you,

Alex.
Received on Fri Jun 15 2012 - 19:34:03 MDT