From: Sergey Burladyan <eshkinkot@gmail.com>
To: Neel Patel <neel.patel@enterprisedb.com>
Cc: pgadmin-hackers <pgadmin-hackers@postgresql.org>,
  Ashesh Vashi <ashesh.vashi@enterprisedb.com>,  Dave Page <dpage@pgadmin.org>
Subject: Re: pgagent unicode support
References: <87a6shyenl.fsf@gmail.com>
	<CA+OCxoxsk-VJOJvw95V+pDN0G79T5-F1jAHB-THsPZd10f0KRg@mail.gmail.com>
	<CACCA4P0PG8y_u9vDwTTL4AHbXOfP1k7joLY4tjLQ40DNpWgyoA@mail.gmail.com>
	<CACCA4P3JvfMmG_LCm8Oeqnp4r=EuJBLuaS2mUieuw+--2ybgFQ@mail.gmail.com>
	<CACCA4P1NgUGZO8PSC6_Js+__pqALrSkd-NGCGWr_OtYptGoqEA@mail.gmail.com>
Date: Wed, 24 Feb 2021 14:23:39 +0300
In-Reply-To: 
 <CACCA4P1NgUGZO8PSC6_Js+__pqALrSkd-NGCGWr_OtYptGoqEA@mail.gmail.com> (Neel
	Patel's message of "Tue, 23 Feb 2021 18:23:57 +0530")
Message-ID: <874ki1yais.fsf@gmail.com>
User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/26.3 (gnu/linux)
MIME-Version: 1.0
Content-Type: text/plain
Precedence: bulk

Neel Patel <neel.patel@enterprisedb.com> writes:

> Do you have any comments for this updated patch ? Let us know ASAP so that
> we can commit it.

Sorry about the delay.

It looks better than my patch, thanks!

There is only two comments, about wcstombs_s and about setlocale.

About win32 wcstombs_s:
+        wcstombs_s(&charsConverted, mbs, mb_len + 10, wchar_str, mb_len + 1);

https://docs.microsoft.com/ru-ru/cpp/c-runtime-library/reference/wcstombs-s-wcstombs-s-l?view=msvc-160
>> sizeInBytes
>> The size in bytes of the mbstr buffer.
>> count
>> The maximum number of bytes to store in the mbstr buffer, not including
>> the terminating null character

Maybe it should look something like this:
+        wcstombs_s(&charsConverted, mbs, mb_len + 1, wchar_str, mb_len);

About setlocale, I think you missed it in your patch:
diff --git a/unix.cpp b/unix.cpp
index 9a41e38..d4b0d3d 100644
--- a/unix.cpp
+++ b/unix.cpp
@@ -155,6 +155,8 @@ static void daemonize(void)
 
 int main(int argc, char **argv)
 {
+	setlocale(LC_ALL, "");
+
 	std::wstring executable;
 	executable.assign(CharToWString(argv[0]));

-- 
Sergey Burladyan