Обсуждение: [PATCH v1] Replace sprintf() with snprintf() in libpq for safety Anexo: o arquivo
[PATCH v1] Replace sprintf() with snprintf() in libpq for safety Anexo: o arquivo
От
Thiago Caserta
Дата:
Hi hackers,
Attached is a patch that converts several sprintf() calls to snprintf() in libpq client library code. While the existing buffers are currently sized correctly, using snprintf() provides an additional safety net against potential buffer overflows and is consistent with the project's general direction of preferring bounded string operations.
Changes:
- fe-auth.c: SSPI target string construction
- fe-connect.c: client encoding query formatting
- fe-exec.c: notice message formatting
- fe-print.c: format string construction
- win32.c: Windows socket error messages
The patch applies cleanly against current HEAD (dd5716f3c74) and passes git diff --check with no whitespace issues. No functional changes are introduced (this is a safety hardening change only).
Best regards,
Thiago Caserta
Вложения
Re: [PATCH v1] Replace sprintf() with snprintf() in libpq for safety Anexo: o arquivo
От
Álvaro Herrera
Дата:
On 2026-Mar-24, Thiago Caserta wrote: > Hi hackers, > > Attached is a patch that converts several sprintf() calls to > snprintf() in libpq client library code. While the existing buffers > are currently sized correctly, using snprintf() provides an > additional safety net against potential buffer overflows and is > consistent with the project's general direction of preferring bounded > string operations. I'm not sure we should take a patch with a tag attributing authorship to an LLM owned by a commercial entity. I think some US court already ruled that LLMs aren't entitled to copyright for works of art, so I suspect we don't need to make any such attribution. On the other hand, the proposed patch is quite dumb, since at least the first hunk shows duplicated strlen(). So it's not clear to me that this patch really achieves the claimed "additional safety net". Do we really want to be accepting code patches written by tools that make the most obvious code worse than before? I am quite scared of the quality of code of medium complexity written by this tool. -- Álvaro Herrera PostgreSQL Developer — https://www.EnterpriseDB.com/ "Los trabajadores menos efectivos son sistematicamente llevados al lugar donde pueden hacer el menor daño posible: gerencia." (El principio Dilbert)
Re: [PATCH v1] Replace sprintf() with snprintf() in libpq for safety Anexo: o arquivo
От
"David G. Johnston"
Дата:
On Thu, Mar 26, 2026 at 4:33 PM Álvaro Herrera <alvherre@kurilemu.de> wrote:
On 2026-Mar-24, Thiago Caserta wrote:
> Attached is a patch that converts several sprintf() calls to
> snprintf() in libpq client library code.
I'm not sure we should take a patch with a tag attributing authorship to
an LLM owned by a commercial entity.
Agreed. As with a book author, any bad code, decisions, or other mistakes are solely the fault of the submitting author. As is the good stuff. Ideally the author has confirmed it is good (in their own opinion) since they expect others to do so as well as part of the review and commit process.
It is in fact a reputational thing for authors to take full ownership of what they submit.
Do we really want to be accepting code patches written by tools that
make the most obvious code worse than before? I am quite scared of the
quality of code of medium complexity written by this tool.
I'd say take this as an opportunity to teach (or not) just as if the author of patch claimed to write the entire thing without AI assistance. But it would definitely be reasonable for a hastily produced patch that doesn't pass muster to be hastily rejected on such grounds. We have plenty to review and if this patch wouldn't have existed without LLM assistance then unless it sparks the interest in someone to go and clean it up anyway we are not really any worse off being quick to state that it doesn't meet our standards.
Otherwise, while there is a patch, maybe just treat it as a simple suggestion with an example.
David J.