Re: Common function for percent placeholder replacement

2023-01-11 Thread Peter Eisentraut
On 11.01.23 19:54, Nathan Bossart wrote: On Wed, Jan 11, 2023 at 11:09:01AM +0100, Peter Eisentraut wrote: committed with that fixed While rebasing my recovery modules patch set, I noticed a couple of small things that might be worth cleaning up. committed, thanks

Re: Common function for percent placeholder replacement

2023-01-11 Thread Nathan Bossart
On Wed, Jan 11, 2023 at 11:09:01AM +0100, Peter Eisentraut wrote: > committed with that fixed While rebasing my recovery modules patch set, I noticed a couple of small things that might be worth cleaning up. -- Nathan Bossart Amazon Web Services: https://aws.amazon.com diff --git

Re: Common function for percent placeholder replacement

2023-01-11 Thread Peter Eisentraut
On 09.01.23 18:53, Nathan Bossart wrote: + nativePath = pstrdup(path); + make_native_path(nativePath); + nativePath = pstrdup(xlogpath); + make_native_path(nativePath); Should these be freed? committed with that fixed

Re: Common function for percent placeholder replacement

2023-01-09 Thread Nathan Bossart
On Mon, Jan 09, 2023 at 09:36:12AM +0100, Peter Eisentraut wrote: > On 04.01.23 01:37, Nathan Bossart wrote: >> On Tue, Dec 20, 2022 at 06:30:40AM +0100, Peter Eisentraut wrote: >> > + * A value may be NULL. If the corresponding placeholder is found in the >> > + * input string, the whole

Re: Common function for percent placeholder replacement

2023-01-09 Thread Peter Eisentraut
09:03:30 +0100 Subject: [PATCH v4] Common function for percent placeholder replacement There are a number of places where a shell command is constructed with percent-placeholders (like %x). It's cumbersome to have to open-code this several times. This factors out this logic into a separate fu

Re: Common function for percent placeholder replacement

2023-01-03 Thread Nathan Bossart
In general, +1. On Tue, Dec 20, 2022 at 06:30:40AM +0100, Peter Eisentraut wrote: > (Still need to think about Robert's comment about lack of error context.) Would adding the name of the GUC be sufficient? ereport(ERROR, (errmsg("could not build %s", guc_name),

Re: Common function for percent placeholder replacement

2022-12-20 Thread Corey Huinker
> > How about this new one with variable arguments? I like this a lot, but I also see merit in Alvaro's PERCENT_OPT variadic, which at least avoids the two lists getting out of sync. Initially, I was going to ask that we have shell-quote-safe equivalents of whatever fixed parameters we baked

Re: Common function for percent placeholder replacement

2022-12-19 Thread Peter Eisentraut
error context.) From 76dbfd291313e610aadb0e01eb46f1b0113f2c0f Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Tue, 20 Dec 2022 06:27:42 +0100 Subject: [PATCH v3] Common function for percent placeholder replacement There are a number of places where a shell command is constructed with percent-placeholders (like %x). It'

Re: Common function for percent placeholder replacement

2022-12-19 Thread Robert Haas
On Mon, Dec 19, 2022 at 3:13 AM Peter Eisentraut wrote: > I agree. Here is an updated patch with the error checking included. Nice, but I think something in the error report needs to indicate what caused the problem exactly. As coded, I think the user would have to guess which GUC caused the

Re: Common function for percent placeholder replacement

2022-12-19 Thread Alvaro Herrera
On 2022-Dec-19, Peter Eisentraut wrote: > On 14.12.22 18:05, Justin Pryzby wrote: > > On Wed, Dec 14, 2022 at 08:31:02AM +0100, Peter Eisentraut wrote: > > > + return replace_percent_placeholders(base_command, "df", (const char > > > *[]){target_detail, filename}); > > > > This is a "compound

Re: Common function for percent placeholder replacement

2022-12-19 Thread Peter Eisentraut
On 14.12.22 18:05, Justin Pryzby wrote: On Wed, Dec 14, 2022 at 08:31:02AM +0100, Peter Eisentraut wrote: + return replace_percent_placeholders(base_command, "df", (const char *[]){target_detail, filename}); This is a "compound literal", which I gather is required by C99. But I don't

Re: Common function for percent placeholder replacement

2022-12-19 Thread Peter Eisentraut
] Common function for percent placeholder replacement There are a number of places where a shell command is constructed with percent-placeholders (like %x). It's cumbersome to have to open-code this several times. This factors out this logic into a separate function. This also allows us to ensure

Re: Common function for percent placeholder replacement

2022-12-14 Thread Tom Lane
Justin Pryzby writes: > On Wed, Dec 14, 2022 at 08:31:02AM +0100, Peter Eisentraut wrote: >> +return replace_percent_placeholders(base_command, "df", (const char >> *[]){target_detail, filename}); > This is a "compound literal", which I gather is required by C99. > But I don't think that's

Re: Common function for percent placeholder replacement

2022-12-14 Thread Justin Pryzby
On Wed, Dec 14, 2022 at 08:31:02AM +0100, Peter Eisentraut wrote: > + return replace_percent_placeholders(base_command, "df", (const char > *[]){target_detail, filename}); This is a "compound literal", which I gather is required by C99. But I don't think that's currently being exercised, so

Re: Common function for percent placeholder replacement

2022-12-14 Thread Robert Haas
On Wed, Dec 14, 2022 at 2:31 AM Peter Eisentraut wrote: > There are a number of places where a shell command is constructed with > percent-placeholders (like %x). First, it's obviously cumbersome to > have to open-code this several times. Second, each of those pieces of > code silently encodes

Common function for percent placeholder replacement

2022-12-13 Thread Peter Eisentraut
ments.From e0e44dbffafa50bf15bca6a03e98783a82ce072c Mon Sep 17 00:00:00 2001 From: Peter Eisentraut Date: Wed, 14 Dec 2022 08:17:25 +0100 Subject: [PATCH v1] Common function for percent placeholder replacement There are a number of places where a shell command is constructed with percent-placeholder