Skip site navigation (1) Skip section navigation (2)

Re: REVIEW: Determining client_encoding from client locale

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:44:39
Message-ID: AANLkTi=SEutmvgW2NWaPu9NVij=48nm=qp-CwwoppZM5@mail.gmail.com (view raw or flat)
Thread:
Lists: pgsql-hackers
On Wed, Feb 2, 2011 at 5:22 AM, Ibrar Ahmed <ibrar(dot)ahmad(at)gmail(dot)com> wrote:

> 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).
>
>
I got the point, why we are doing this, just to calculate the length. Sorry
for this point.



> 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
>
>
>


-- 
   Ibrar Ahmed

In response to

pgsql-hackers by date

Next:From: Kevin GrittnerDate: 2011-02-02 14:11:14
Subject: Re: SSI patch version 14
Previous:From: Magnus HaganderDate: 2011-02-02 13:26:57
Subject: Some more walsender "metadata"

Privacy Policy | About PostgreSQL
Copyright © 1996-2014 The PostgreSQL Global Development Group