Re: REVIEW: Determining client_encoding from client locale - Mailing list pgsql-hackers
| From | Ibrar Ahmed |
|---|---|
| Subject | Re: REVIEW: Determining client_encoding from client locale |
| Date | |
| Msg-id | AANLkTinJX4=sfTbYOK+zXiRZfFWqzvGv-3-7LfhkABtr@mail.gmail.com Whole thread Raw |
| In response to | Re: REVIEW: Determining client_encoding from client locale (Heikki Linnakangas <heikki.linnakangas@enterprisedb.com>) |
| Responses |
Re: REVIEW: Determining client_encoding from client locale
Re: REVIEW: Determining client_encoding from client locale |
| List | pgsql-hackers |
Hi!,
I have reviewed/tested this patch.
OS = "Linux ubuntu 2.6.35-22-generic #33-Ubuntu SMP Sun Sep 19 20:34:50 UTC 2010 i686 GNU/Linux"
PostgreSQL Version = Head (9.1devel)
Patch gives the desired results(still testing), but couple of questions with this portion of the code.
if (conn->client_encoding_initial && conn->client_encoding_initial[0])
{
if (packet)
strcpy(packet + packet_len, "client_encoding");
packet_len += strlen("client_encoding") + 1;
if (packet)
strcpy(packet + packet_len, conn->client_encoding_initial);
packet_len += strlen(conn->client_encoding_initial) + 1;
}
Is there any reason to check "packet" twice?.
And suppose "packet" is null then we will not append "client_encoding" into the string but will increase the length(looks intentional! like in ADD_STARTUP_OPTION).
In my point code should be like this
In my point code should be like this
if (conn->client_encoding_initial && conn->client_encoding_initial[0])
{
if (packet)
{
strcpy(packet + packet_len, "client_encoding");
packet_len += strlen("client_encoding") + 1;
strcpy(packet + packet_len, conn->client_encoding_initial);
packet_len += strlen(conn->client_encoding_initial) + 1;
}
}
{
if (packet)
{
strcpy(packet + packet_len, "client_encoding");
packet_len += strlen("client_encoding") + 1;
strcpy(packet + packet_len, conn->client_encoding_initial);
packet_len += strlen(conn->client_encoding_initial) + 1;
}
}
BTW why you have not used "ADD_STARTUP_OPTION"?
I will test this patch on Windows and will send results.
--
Ibrar Ahmed
Ibrar Ahmed
On Sun, Jan 30, 2011 at 10:56 AM, Heikki Linnakangas <heikki.linnakangas@enterprisedb.com> wrote:
On 29.01.2011 19:23, Peter Eisentraut wrote:The other options send by the OPTION_SEND/WAIT machinery come from environment variables, there's a getenv() call where the SETENV_STATE_OPTION_SEND state is handled. It would be a bit awkward to shoehorn client_encoding into that.> Also, do we really need a new set of states for this..? I would haveDon't know. Maybe Heikki could comment; he wrote that initially.
> thought, just reading through the patch, that we could use the existing
> OPTION_SEND/OPTION_WAIT states..
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
--
Ibrar Ahmed
pgsql-hackers by date: