Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Wed, Aug 24, 2016 at 9:55 AM, Alvaro Herrera wrote: > Ryan Murphy wrote: >> Thanks for committing it Tom! Thank you all for your help. >> >> I really like the Postgres project! If there's anything that especially >> needs worked on let me know, I'd love to help. > >

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Ryan Murphy wrote: > Thanks for committing it Tom! Thank you all for your help. > > I really like the Postgres project! If there's anything that especially > needs worked on let me know, I'd love to help. https://wiki.postgresql.org/wiki/Todo -- Álvaro Herrera

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Thanks for committing it Tom! Thank you all for your help. I really like the Postgres project! If there's anything that especially needs worked on let me know, I'd love to help. Best, Ryan

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Mon, Aug 22, 2016 at 8:45 PM, Tom Lane wrote: > Likely it would be better to refactor all of this so we get just one > reference to libpq and one reference to libpgport, but that'd require > a more thoroughgoing cleanup than you have here. (Now that I think > about it,

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Michael Paquier writes: > On Sun, Aug 21, 2016 at 3:02 AM, Tom Lane wrote: >> I looked into this and soon found that fe_utils/string_utils.o has >> dependencies on libpq that are much wider than just pqexpbuffer :-(. > pqexpbuffer.c is an

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Sun, Aug 21, 2016 at 3:02 AM, Tom Lane wrote: > I wrote: >> A bigger issue here is that it seems fundamentally wrong for initdb to be >> including libpq, because it surely is never meant to be communicating >> with a running postmaster. Not sure what to do about that. We

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

> > I looked into this and soon found that fe_utils/string_utils.o has > dependencies on libpq that are much wider than just pqexpbuffer :-(. > It might be a project to think about sorting that out sometime, but it > looks like it would be an awful lot of work just to avoid having initdb > depend

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

I wrote: > A bigger issue here is that it seems fundamentally wrong for initdb to be > including libpq, because it surely is never meant to be communicating > with a running postmaster. Not sure what to do about that. We could > consider moving pqexpbuffer out of libpq into fe_utils, but I

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Michael Paquier writes: > Regarding your patch, with a bit of clean up it gives the attached. This fails to build for me, with gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Wendif-labels -Wmissing-format-attribute -Wformat-security

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Sat, Aug 20, 2016 at 8:26 AM, Michael Paquier wrote: > On Sat, Aug 20, 2016 at 4:39 AM, Ryan Murphy > wrote: > > Here is another version of my initdb shell quoting patch. I have removed > > the unnecessary {} block. I also ran pgindent on

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Sat, Aug 20, 2016 at 4:39 AM, Ryan Murphy wrote: > Here is another version of my initdb shell quoting patch. I have removed > the unnecessary {} block. I also ran pgindent on the code prior to creating > the patch. Could you please *not* top-post? This breaks the

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Here is another version of my initdb shell quoting patch. I have removed the unnecessary {} block. I also ran pgindent on the code prior to creating the patch. On Thu, Aug 18, 2016 at 3:50 PM, Ryan Murphy wrote: > > > On Thu, Aug 18, 2016 at 3:44 PM, Tom Lane

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Thu, Aug 18, 2016 at 3:44 PM, Tom Lane wrote: > Ryan Murphy writes: > > On Thu, Aug 18, 2016 at 3:22 PM, Robert Haas > wrote: > +{ /* pg_ctl command w path, properly quoted */ > +PQExpBuffer

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Ryan Murphy writes: > On Thu, Aug 18, 2016 at 3:22 PM, Robert Haas wrote: +{ /* pg_ctl command w path, properly quoted */ +PQExpBuffer pg_ctl_path = createPQExpBuffer(); +printfPQExpBuffer(pg_ctl_path,

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Thu, Aug 18, 2016 at 3:22 PM, Robert Haas wrote: > On Thu, Aug 18, 2016 at 4:15 PM, Andres Freund wrote: > > On 2016-08-18 16:11:27 -0400, Robert Haas wrote: > >> On Wed, Aug 17, 2016 at 11:18 PM, Andres Freund > wrote: > >> >

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Thu, Aug 18, 2016 at 4:15 PM, Andres Freund wrote: > On 2016-08-18 16:11:27 -0400, Robert Haas wrote: >> On Wed, Aug 17, 2016 at 11:18 PM, Andres Freund wrote: >> > On August 17, 2016 8:15:56 PM PDT, Michael Paquier >> >

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On 2016-08-18 16:11:27 -0400, Robert Haas wrote: > On Wed, Aug 17, 2016 at 11:18 PM, Andres Freund wrote: > > On August 17, 2016 8:15:56 PM PDT, Michael Paquier > > wrote: > > > >>+{ /* pg_ctl command w path, properly quoted */ > >>+

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Wed, Aug 17, 2016 at 11:18 PM, Andres Freund wrote: > On August 17, 2016 8:15:56 PM PDT, Michael Paquier > wrote: > >>+{ /* pg_ctl command w path, properly quoted */ >>+PQExpBuffer pg_ctl_path = createPQExpBuffer(); >>+

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On August 17, 2016 8:15:56 PM PDT, Michael Paquier wrote: >+{ /* pg_ctl command w path, properly quoted */ >+PQExpBuffer pg_ctl_path = createPQExpBuffer(); >+printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl", >+bin_dir, >+

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Thu, Aug 18, 2016 at 11:35 AM, Ryan Murphy wrote: Be careful of top-posting, this is not this ML style: http://www.idallen.com/topposting.html >> I think that's actually a good thing to forbid. > > I think I agree Andres, there are already comments in the

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

> I think that's actually a good thing to forbid. I think I agree Andres, there are already comments in the appendShellString function to this effect - they say that CR/LF chars in a file name are mostly used for malicious hacking attempts anyways - I know I've hardly ever needed a newline in a

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On 2016-08-18 09:14:44 +0900, Michael Paquier wrote: > On Thu, Aug 18, 2016 at 12:21 AM, Ryan Murphy wrote: > > I have created a better patch (attached) that correctly escapes the shell > > arguments using PQExpBufferStr and the appendShellString function, as per > >

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Thu, Aug 18, 2016 at 9:30 AM, Ryan Murphy wrote: (please avoid top-posting) >> As far as I know, it is perfectly possible to have LF/CR in a path >> name (that's bad practice btw...), and your patch would make initdb >> fail in such cases. Do we want to authorize that?

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

That's a fair point Michael. I would be willing to make such a change, but since c doesn't have optional function arguments I'm not sure the least intrusive way to do that. Do you have a suggestion? On Wednesday, August 17, 2016, Michael Paquier wrote: > On Thu, Aug

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Thu, Aug 18, 2016 at 12:21 AM, Ryan Murphy wrote: > I have created a better patch (attached) that correctly escapes the shell > arguments using PQExpBufferStr and the appendShellString function, as per > Michael and Andres' suggestions. > > Further suggestions welcome of

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

I have created a better patch (attached) that correctly escapes the shell arguments using PQExpBufferStr and the appendShellString function, as per Michael and Andres' suggestions. Further suggestions welcome of course. Ryan On Wed, Aug 17, 2016 at 8:28 AM, Ryan Murphy

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

That makes sense, Michael and Andres. I started to make a solution that uses a PQExpBuffer, appendShellString, etc. I think it will work just fine, but I think I need to alter the Makefile as well, to get initdb.c to be compiled using -L../../../src/fe_utils -lpgfeutils. Otherwise I am having

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Wed, Aug 17, 2016 at 8:05 AM, Andres Freund wrote: > ISTM that the correct fix would be to actually introduce something like > quote_path_for_shell() which either adds proper quotes, or fails if > that'd be hard (e.g. if the path contains quotes, and we're on > windows).

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Hi, On 2016-08-14 10:12:45 -0500, Ryan Murphy wrote: > Hello Postgres Team! > but this command did not work for me because my data directory > contained a space. The src/bin/initdb/initdb.c source code > did already have a QUOTE_PATH constant, but it was the empty > string for non-windows cases.

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

I've submitted my patch to Commitfest 2016-09. https://commitfest.postgresql.org/10/718/ My username on postgresql.org is murftown On Tue, Aug 16, 2016 at 1:02 AM, Ryan Murphy wrote: > Ok, I'll do that! > Thanks Michael! > Ryan > > > On Monday, August 15, 2016, Michael

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Ok, I'll do that! Thanks Michael! Ryan On Monday, August 15, 2016, Michael Paquier wrote: > On Mon, Aug 15, 2016 at 12:12 AM, Ryan Murphy > wrote: > > This is to fix an issue that came up for me when running initdb. > > > > At

Re: [HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

On Mon, Aug 15, 2016 at 12:12 AM, Ryan Murphy wrote: > This is to fix an issue that came up for me when running initdb. > > At the end of a successful initdb it says: > > Success. You can now start the database server using: > pg_ctl -D /some/path/to/data -l

[HACKERS] Patch: initdb: "'" for QUOTE_PATH (non-windows)

Hello Postgres Team! This is to fix an issue that came up for me when running initdb. At the end of a successful initdb it says: Success. You can now start the database server using: pg_ctl -D /some/path/to/data -l logfile start but this command did not work for me because my data