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 AANLkTi=SEutmvgW2NWaPu9NVij=48nm=qp-CwwoppZM5@mail.gmail.com
Whole thread Raw
In response to Re: REVIEW: Determining client_encoding from client locale  (Ibrar Ahmed <ibrar.ahmad@gmail.com>)
List pgsql-hackers


On Wed, Feb 2, 2011 at 5:22 AM, Ibrar Ahmed <ibrar.ahmad@gmail.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.linnakangas@enterprisedb.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers



--
   Ibrar Ahmed





--
   Ibrar Ahmed


pgsql-hackers by date:

Previous
From: Steve Singer
Date:
Subject: Re: pl/python explicit subtransactions
Next
From: "Kevin Grittner"
Date:
Subject: Re: SSI patch version 14