Re: [PATCH 2/2] gettext.h: add parentheses around N_ expansion

From: Kyle J. McKay <mackyle@gmail.com>
Date: Jan 6, 2015, 14:38 UTC
Message-ID: <694DC462-40E3-4081-8616-64483FCD0D77@gmail.com>
In-Reply-To: <54ABE205.70801@ramsay1.demon.co.uk>

On Jan 6, 2015, at 05:24, Ramsay Jones wrote:

Show 32 quoted lines

> On 06/01/15 10:34, Kyle J. McKay wrote:
>> Avoid this by adding parentheses around the expansion of the
>> N_ macro so that instead of ending up with two adjacent strings
>> that are then combined by the preprocessor, two adjacent strings
>> surrounded by parentheses result instead which causes a compile
>> error so the mistake can be quickly found and corrected.
>>
>> Signed-off-by: Kyle J. McKay <mackyle@gmail.com>
>> ---
>> This patch is optional, but prevents the problem fixed by 1/2
>> from recurring.
>>
>> gettext.h | 2 +-
>> 1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/gettext.h b/gettext.h
>> index 7671d09d..d11a4139 100644
>> --- a/gettext.h
>> +++ b/gettext.h
>> @@ -63,6 +63,6 @@ const char *Q_(const char *msgid, const char  
>> *plu, unsigned long n)
>> }
>>
>> /* Mark msgid for translation but do not translate it. */
>> -#define N_(msgid) msgid
>> +#define N_(msgid) (msgid)
>>
>> #endif
>>
>
> Hmm, see commit 642f85faa ("i18n: avoid parenthesized string as
> array initializer", 07-04-2011), for a counter-point. :-P

Oh interesting. So the bug bug fixed by 1/2 must have crept in after that change then.

Even clang -ansi -pedantic doesn't seem to complain about this. And ("a") is just as much a constant expression as "a". Are you sure it's not just a tcc bug?

In any case, 642f85faa is talking about this:

   static const char ignore_error[] = ("something");

which is assigning a const char * to a const char [].

But builtin/log.c uses this:

   static const char * const builtin_log_usage[] = {("something",  
"else"};

Which is assigning a const char * to a const char * const.

Anyhow, if it breaks some existing compiler that works now, it shouldn't be picked up. But it would sure be nice to have something to prevent a recurrence of the bug 1/2 fixes. Perhaps the (msgid) version should be enabled when __GNUC__ is defined -- that should be compatible and enough folks use that it would catch such a problem early.

-Kyle