Re: Review: psql include file using relative path

From: Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Josh Kupershmidt <schmiddy(at)gmail(dot)com>, pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: Review: psql include file using relative path
Date: 2011-07-06 17:07:35
Message-ID: CABwTF4XOAVLZO+M4t1Pn09qirepXNojz62BVxB_SGnMhm5H_QA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Wed, Jul 6, 2011 at 11:58 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:

> On Mon, Jun 6, 2011 at 10:11 PM, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>
> wrote:
> > On Mon, Jun 6, 2011 at 9:48 PM, Josh Kupershmidt <schmiddy(at)gmail(dot)com>
> wrote:
> >>
> >> On Sun, Jun 5, 2011 at 8:16 PM, Gurjeet Singh <singh(dot)gurjeet(at)gmail(dot)com>
> >> wrote:
> >> > Attached an updated patch.
> >> >
> >> > If you find it ready for committer, please mark it so in the
> commitfest
> >> > app.
> >>
> >> I can't find anything further to nitpick with this patch, and have
> >> marked it Ready For Committer in the CF. Thanks for your work on this,
> >> I am looking forward to the feature.
> >
> > Thanks for your reviews and perseverance :)
>
> I committed this after substantial further revisions:
>
> - I rewrote the changes to process_file() to use the pathname-handling
> functions in src/port, rather custom code. Along the way, relpath
> became a constant-size buffer, which should be OK since
> join_pathname_components() knows about MAXPGPATH. This has what I
> consider to be a useful side effect of not calling pg_malloc() here,
> which means we don't have to remember to free the memory.
>
> - I added a safeguard against someone doing something like "\ir E:foo"
> on Windows. Although that's not an absolute path, for purposes of \ir
> it needs to be treated as one. I don't have a Windows build
> environment handy so someone may want to test that I haven't muffed
> this.
>
> - I rewrote the documentation and a number of the comments to be (I
> hope) more clear.
>
> - I reverted some unnecessary whitespace changes in exec_command().
>
> - As proposed, the patch declared process_file with a non-constant
> initialized and then declared another variable after that. I believe
> some old compilers will barf on that. Since it isn't needed in that
> block anyway, I moved it to an inner block.
>
> - I incremented the pager line count for psql's help.
>

Thank you Robert and Josh for all the help.

--
Gurjeet Singh
EnterpriseDB Corporation
The Enterprise PostgreSQL Company

In response to

Browse pgsql-hackers by date

  From Date Subject
Next Message Alvaro Herrera 2011-07-06 17:09:11 Re: [v9.2] DROP Reworks Part.1 - Consolidate routines to handle DropStmt
Previous Message Jeff Davis 2011-07-06 17:04:00 Re: Range Types, constructors, and the type system