From: | Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com> |
---|---|
To: | Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> |
Cc: | Peter Eisentraut <peter_e(at)gmx(dot)net>, Stephen Frost <sfrost(at)snowman(dot)net>, Jaime Casanova <jaime(at)2ndquadrant(dot)com>, Magnus Hagander <magnus(at)hagander(dot)net>, pgsql-hackers(at)postgresql(dot)org, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Bruce Momjian <bruce(at)momjian(dot)us>, Itagaki Takahiro <itagaki(dot)takahiro(at)oss(dot)ntt(dot)co(dot)jp> |
Subject: | Re: REVIEW: Determining client_encoding from client locale |
Date: | 2011-02-02 13:22:58 |
Message-ID: | AANLkTinJX4=sfTbYOK+zXiRZfFWqzvGv-3-7LfhkABtr@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
*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;
}
}*
*
*
*
*
BTW why you have not used "ADD_STARTUP_OPTION"?
I will test this patch on Windows and will send results.
--
Ibrar Ahmed
On Sun, Jan 30, 2011 at 10:56 AM, Heikki Linnakangas <
heikki(dot)linnakangas(at)enterprisedb(dot)com> wrote:
> On 29.01.2011 19:23, Peter Eisentraut wrote:
>
>> > Also, do we really need a new set of states for this..? I would have
>>> > thought, just reading through the patch, that we could use the
>>> existing
>>> > OPTION_SEND/OPTION_WAIT states..
>>>
>> Don't know. Maybe Heikki could comment; he wrote that initially.
>>
>
> 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.
>
> --
> Heikki Linnakangas
> EnterpriseDB http://www.enterprisedb.com
>
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers
>
--
Ibrar Ahmed
From | Date | Subject | |
---|---|---|---|
Next Message | Magnus Hagander | 2011-02-02 13:26:57 | Some more walsender "metadata" |
Previous Message | Markus Wanner | 2011-02-02 13:22:24 | Re: SSI patch version 14 |