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? (I'm not sure I buy pg_read_line's use of perror in the backend case. Maybe this is only okay because the backend doesn't use this code?) pg_get_line says caller can distinguish error from no-input-before-EOF with ferror(), but pipe_read_line does no such thing. I wonder what happens if an NFS mount containing a file being read disappears in the middle of reading it, for example ... will we think that we completed reading the file, rather than erroring out? -- Álvaro Herrera Breisgau, Deutschland — https://www.EnterpriseDB.com/ "That sort of implies that there are Emacs keystrokes which aren't obscure. I've been using it daily for 2 years now and have yet to discover any key sequence which makes any sense." (Paul Thomas)