Re: pipe_read_line for reading arbitrary strings

From: Peter Eisentraut <peter(at)eisentraut(dot)org>
To: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, Daniel Gustafsson <daniel(at)yesql(dot)se>
Cc: Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: pipe_read_line for reading arbitrary strings
Date: 2024-03-06 09:07:27
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-hackers

On 22.11.23 13:47, Alvaro Herrera wrote:
> On 2023-Mar-07, Daniel Gustafsson wrote:
>> The attached POC diff replace fgets() with pg_get_line(), which may not be an
>> Ok way to cross the streams (it's clearly not a great fit), but as a POC it
>> provided a neater interface for reading one-off lines from a pipe IMO. Does
>> anyone else think this is worth fixing before too many callsites use it, or is
>> this another case of my fear of silent subtle truncation bugs? =)
> I think this is generally a good change.
> I think pipe_read_line should have a "%m" in the "no data returned"
> error message. pg_read_line is careful to retain errno (and it was
> already zero at start), so this should be okay ... or should we set
> errno again to zero after popen(), even if it works?

Is this correct? The code now looks like this:

line = pg_get_line(pipe_cmd, NULL);

if (line == NULL)
if (ferror(pipe_cmd))
_("could not read from command \"%s\": %m"), cmd);
_("no data was returned by command \"%s\": %m"),

We already handle the case where an error happened in the first branch,
so there cannot be an error set in the second branch (unless something
nonobvious is going on?).

It seems to me that if the command being run just happens to print
nothing but is otherwise successful, this would print a bogus error code
(or "Success")?

In response to


Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2024-03-06 09:10:35 Re: [PoC] Improve dead tuple storage for lazy vacuum
Previous Message Bertrand Drouvot 2024-03-06 09:05:59 Re: Missing LWLock protection in pgstat_reset_replslot()