Re: Patch : PGPASSFILE fix

From: Prasad <prasad(dot)s(at)mail(dot)com>
To: "Ashesh Vashi" <ashesh(dot)vashi(at)enterprisedb(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: Patch : PGPASSFILE fix
Date: 2015-03-04 07:44:13
Message-ID: trinity-e1c50e05-2ae3-4ebe-bb2d-2b021065f210-1425455052907@3capp-mailcom-lxa05
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgadmin-hackers

Ashesh,

Thanks for reviewing patch, 
Code I have removed in I think, was switch statement inside if condition, which doesn't make sense.
ie.
if (var == 2)
{
switch (var)
case 2:
.....
break;
}

that's why I removed it, because it's redundant.
About creation of directory, I'm not sure if this validation is required. Existing code creates directory postgresql (only on windows) according to http://www.postgresql.org/docs/9.3/static/libpq-pgpass.html , and it doesn't create file. I'm not sure whether this kind of validation is expected in this function.

regards,
Prasad

Sent: Wednesday, March 04, 2015 at 7:15 AM
From: "Ashesh Vashi" <ashesh(dot)vashi(at)enterprisedb(dot)com>
To: Prasad <prasad(dot)s(at)mail(dot)com>
Cc: pgadmin-hackers <pgadmin-hackers(at)postgresql(dot)org>
Subject: Re: [pgadmin-hackers] Patch : PGPASSFILE fix

Hi Prasad,
 I see couple of issues with your patch.* Please generate the patch using 'git diff'.
  I could not apply your patch straight forwardly.
  I had to use the patch utility.
 * Please follow the coding style of pgAdmin.
  You can find it at https://wiki.postgresql.org/wiki/PgAdmin_Internals#Coding_Style.* Do not remove any of the existing code.
  It has been kept there keeping in mind about future development extending support of the existing functionality.
  You've removed couple of lines in the sysSettings::GetConfigFile(...) function, which is not good.
 
In your code:* Checked only for PGPASSFILE environment variable.
* Need to check the existence of the file.
* Take required actions (if that file/parent directory does not exists).
    i.e. Create parent directory
 
 

--
Thanks & Regards,

Ashesh Vashi
EnterpriseDB INDIA: Enterprise PostgreSQL Company[http://www.enterprisedb.com]
 
http://www.linkedin.com/in/asheshvashi[http://www.linkedin.com/in/asheshvashi]
 
On Sun, Mar 1, 2015 at 11:08 PM, Prasad <prasad(dot)s(at)mail(dot)com[prasad(dot)s@mail.com]> wrote:
Hi,
 
Find attached fix for reading PGPASSFILE environment variable for pg password file.
 
regards,
Prasad

--
Sent via pgadmin-hackers mailing list (pgadmin-hackers(at)postgresql(dot)org[pgadmin-hackers@postgresql.org])
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgadmin-hackers[http://www.postgresql.org/mailpref/pgadmin-hackers]
 

In response to

Responses

Browse pgadmin-hackers by date

  From Date Subject
Next Message Ashesh Vashi 2015-03-04 07:48:54 Re: Patch : PGPASSFILE fix
Previous Message Ashesh Vashi 2015-03-04 07:15:12 Re: Patch : PGPASSFILE fix