Re: Add Information during standby recovery conflicts

Hi,

On 10/15/20 9:15 AM, Masahiko Sawada wrote:
> CAUTION: This email originated from outside of the organization. Do not click links or open attachments unless you
canconfirm the sender and know the content is safe.
 
>
>
>
> On Thu, 15 Oct 2020 at 14:52, Kyotaro Horiguchi <horikyota.ntt@gmail.com> wrote:
>> At Thu, 15 Oct 2020 14:28:57 +0900, Masahiko Sawada <masahiko.sawada@2ndquadrant.com> wrote in
>>>> ereport(..(errmsg("%s", _("hogehoge")))) results in
>>>> fprintf((translated("%s")), translate("hogehoge")).
>>>>
>>>> So your change (errmsg("%s", gettext_noop("hogehoge")) results in
>>>>
>>>> fprintf((translated("%s")), DONT_translate("hogehoge")).
>>>>
>>>> which leads to a translation problem.
>>>>
>>>> (errmsg(gettext_noop("hogehoge"))
>>> This seems equivalent to (errmsg("hogehoge")), right?
>> Yes and no.  However eventually the two works the same way,
>> "(errmsg(gettext_noop("hogehoge"))" is a shorthand of
>>
>> 1: char *msg = gettext_noop("hogehoge");
>> ...
>> 2: .. (errmsg(msg));
>>
>> That is, the line 1 only registers a message id "hogehoge" and doesn't
>> translate. The line 2 tries to translate the content of msg and it
>> finds the translation for the message id "hogehoge".
> Understood.
>
>>> I think I could understand translation stuff. Given we only report the
>>> const string returned from get_recovery_conflict_desc() without
>>> placeholders, the patch needs to use errmsg_internal() instead while
>>> not changing _() part. (errmsg(get_recovery_conflict_desc())) is not
>>> good (warned by -Wformat-security).
>> Ah, right. we get a complain if no value parameters added. We can
>> silence it by adding a dummy parameter to errmsg, but I'm not sure
>> which is preferable.
> Okay, I'm going to use errmsg_internal() for now until a better idea comes.
>
> I've attached the updated patch that fixed the translation part.

Thanks for reviewing and helping on this patch!

The patch tester bot is currently failing due to:

"proc.c:1290:5: error: ‘standbyWaitStart’ may be used uninitialized in 
this function [-Werror=maybe-uninitialized]"

I've attached a new version with the minor change to fix it.

Bertrand