Re: Patch: initdb: "'" for QUOTE_PATH (non-windows) - Mailing list pgsql-hackers

From Robert Haas
Subject Re: Patch: initdb: "'" for QUOTE_PATH (non-windows)
Date
Msg-id CA+TgmoZ0EPwqDSjAqXYi+-pLv_M1Zo+gEKtEn8Qr7XL=YLZPeA@mail.gmail.com
Whole thread Raw
In response to Re: Patch: initdb: "'" for QUOTE_PATH (non-windows)  (Andres Freund <andres@anarazel.de>)
Responses Re: Patch: initdb: "'" for QUOTE_PATH (non-windows)  (Ryan Murphy <ryanfmurphy@gmail.com>)
List pgsql-hackers
On Thu, Aug 18, 2016 at 4:15 PM, Andres Freund <andres@anarazel.de> wrote:
> On 2016-08-18 16:11:27 -0400, Robert Haas wrote:
>> On Wed, Aug 17, 2016 at 11:18 PM, Andres Freund <andres@anarazel.de> wrote:
>> > On August 17, 2016 8:15:56 PM PDT, Michael Paquier <michael.paquier@gmail.com> wrote:
>> >
>> >>+        { /* pg_ctl command w path, properly quoted */
>> >>+            PQExpBuffer pg_ctl_path = createPQExpBuffer();
>> >>+            printfPQExpBuffer(pg_ctl_path, "%s%spg_ctl",
>> >>+                bin_dir,
>> >>+                (strlen(bin_dir) > 0) ? DIR_SEP : ""
>> >>+            );
>> >>+            appendShellString(start_db_cmd, pg_ctl_path->data);
>> >>+            destroyPQExpBuffer(pg_ctl_path);
>> >>+        }
>> >>
>> >>This is not really project-style to have an independent block. Usually
>> >>those are controlled by for, while or if.
>> >
>> > Besides the comment positioning I'd not say that that is against the usual style, there's a number of such blocks
already. Don't think it's necessarily needed here though...
 
>>
>> Really?  I'd remove such blocks before committing anything, or ask for
>> them to be removed, unless there were some special reason for having
>> them.
>
> Well, reducing the scope of variables *can* be such a reason, no? As I
> said, I don't see any reason here, but in general, it's imo a reasonable
> tool on one's belt.

I think it's worth reducing the scope of variables when that's as
simple as putting them inside a block that you have to create anyway,
but I'm skeptical about the idea that one would create a block just to
reduce the scope of the variables.  I don't think that's our usual
practice, and I would expect the compiler to detect where the variable
is referenced first and last anyway.

-- 
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company



pgsql-hackers by date:

Previous
From: Andres Freund
Date:
Subject: Re: Patch: initdb: "'" for QUOTE_PATH (non-windows)
Next
From: Christian Convey
Date:
Subject: Re: WIP: About CMake v2