On Sat, Nov 01, 2025 at 04:49:02PM -0700, Joshua Shanks wrote: > The conn parameter is placed first, consistent with the object-oriented > convention used throughout libpq (see connectOptions1, > fillPGconn, store_conn_addrinfo, etc.). Sounds OK to me. Nice catch. It's probably not worth bothering in the stable branches as the error is unlikely going to show up. -- Michael
On Sun, Nov 02, 2025 at 09:09:55AM +0900, Michael Paquier wrote: > Sounds OK to me. Nice catch. It's probably not worth bothering in > the stable branches as the error is unlikely going to show up. And, while checking the surroundings, I have noticed that your patch is incomplete: passwordFromFile() is not designed currently in such a way in fe-connect.c so as it is possible for its callers to make a difference between one of the valid cases (like a dbname or user name equal to NULL, service file that cannot be opened, etc.) and an error. The point would be to have pqConnectOptions2() take its oom_error path in the case you are pointing at. It seems to me that the correct answer would be to extend passwordFromFile() with an extra "const char **errmsg", where an error is saved, assign an error string in the routine, check if the error pointer is set when exiting passwordFromFile() in pqConnectOptions2(), and consume the error properly before failing in pqConnectOptions2() with a CONNECTION_BAD. -- Michael
The point would be to have pqConnectOptions2() take its oom_error path
in the case you are pointing at. It seems to me that the correct
answer would be to extend passwordFromFile() with an extra "const char
**errmsg", where an error is saved, assign an error string in the
routine, check if the error pointer is set when exiting
passwordFromFile() in pqConnectOptions2(), and consume the error
properly before failing in pqConnectOptions2() with a CONNECTION_BAD.
On Mon, Nov 03, 2025 at 09:15:25PM -0800, Joshua Shanks wrote: > Thank you for the detailed feedback! This is my first C project > contribution, so I really appreciate you taking the time to explain the > proper error handling pattern. No problem. Applied on HEAD after a few tweaks: * two libpq_gettext() added, * *errmsg set to NULL when entering passwordFromFile(). * Some rewords. -- Michael
Сайт использует файлы cookie для корректной работы и повышения удобства. Нажимая кнопку «Принять» или продолжая пользоваться сайтом, вы соглашаетесь на их использование в соответствии с Политикой в отношении обработки cookie ООО «ППГ», в том числе на передачу данных из файлов cookie сторонним статистическим и рекламным службам. Вы можете управлять настройками cookie через параметры вашего браузера