From: | Daniel Gustafsson <daniel(at)yesql(dot)se> |
---|---|
To: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
Subject: | pipe_read_line for reading arbitrary strings |
Date: | 2023-03-07 22:05:12 |
Message-ID: | DEDF73CE-D528-49A3-9089-B3592FD671A9@yesql.se |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | pgsql-hackers |
When skimming through pg_rewind during a small review I noticed the use of
pipe_read_line for reading arbitrary data from a pipe, the mechanics of which
seemed odd.
Commit 5b2f4afffe6 refactored find_other_exec() and broke out pipe_read_line()
as a static convenience routine for reading a single line of output to catch a
version number. Many years later, commit a7e8ece41 exposed it externally in
order to read a GUC from postgresql.conf using "postgres -C ..". f06b1c598
also make use of it for reading a version string much like find_other_exec().
Funnily enough, while now used for arbitrary string reading the variable is
still "pgver".
Since the function requires passing a buffer/size, and at most size - 1 bytes
will be read via fgets(), there is a truncation risk when using this for
reading GUCs (like how pg_rewind does, though the risk there is slim to none).
If we are going to continue using this for reading $stuff from pipes, maybe we
should think about presenting a nicer API which removes that risk? Returning
an allocated buffer which contains all the output along the lines of the recent
pg_get_line work seems a lot nicer and safer IMO.
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? =)
--
Daniel Gustafsson
Attachment | Content-Type | Size |
---|---|---|
pipe_read_line.diff | application/octet-stream | 4.6 KB |
From | Date | Subject | |
---|---|---|---|
Next Message | Peter Smith | 2023-03-07 22:26:29 | PGDOCS - Replica Identity quotes |
Previous Message | David Rowley | 2023-03-07 21:25:34 | Re: Add support for unit "B" to pg_size_pretty() |