Replace references to malloc() in libpq documentation with generic language
The commit message in the attached patch provides the reasoning, as follows:
The user does not benefit from knowing that libpq allocates some/all memory
using malloc(). Mentioning malloc() here has a few downsides, and almost no
benefits.
All the relevant mentions of malloc() are followed by an explicit
instruction to use PQfreemem() to free the resulting objects. So the
docs perform the sufficient job of educating the user on how to properly
free the memory. But these mentions of malloc() may still lead an
inexperienced or careless user to (wrongly) believe that they may use
free() to free the resulting memory. They will be in a lot of pain until
they learn that (when linked statically) libpq's malloc()/free() cannot
be mixed with malloc()/free() of whichever malloc() library the client
application is being linked with.
Another downside of explicitly mentioning that objects returned by libpq
functions are allocated with malloc(), is that it exposes the implementation
details of libpq to the users. Such details are best left unmentioned so that
these can be freely changed in the future without having to worry about its
effect on client applications.
Whenever necessary, it is sufficient to tell the user that the objects/memory
returned by libpq functions is allocated on the heap. That is just enough
detail for the user to realize that the relevant object/memory needs to be
freed; and the instructions that follow mention to use PQfreemem() to free such
memory.
One mention of malloc is left intact, because that mention is unrelated to how
the memory is allocated, or how to free it.
In passing, slightly improve the language of PQencryptPasswordConn()
documentation.
Best regards,
Gurjeet
http://Gurje.et
Attachments:
v1-0001-Replace-references-to-malloc-in-libpq-documentati.patchapplication/octet-stream; name=v1-0001-Replace-references-to-malloc-in-libpq-documentati.patchDownload
From 156048b6fd448ca6c89b67d390decb91cb164788 Mon Sep 17 00:00:00 2001
From: Gurjeet Singh <gurjeet@singh.im>
Date: Mon, 23 Oct 2023 21:13:42 -0700
Subject: [PATCH v1] Replace references to malloc() in libpq documentation with
generic language
The user does not benefit from knowing that libpq allocates some/all memory
using malloc(). Mentioning malloc() here has a few downsides, and almost no
benefits.
All the relevant mentions of malloc() are followed by an explicit
instruction to use PQfreemem() to free the resulting objects. So the
docs perform the sufficient job of educating the user on how to properly
free the memory. But these mentions of malloc() may still lead an
inexperienced or careless user to (wrongly) believe that they may use
free() to free the resulting memory. They will be in a lot of pain until
they learn that (when linked statically) libpq's malloc()/free() cannot
be mixed with malloc()/free() of whichever malloc() library the client
application is being linked with.
Another downside of explicitly mentioning that objects returned by libpq
functions are allocated with malloc(), is that it exposes the implementation
details of libpq to the users. Such details are best left unmentioned so that
these can be freely changed in the future without having to worry about its
effect on client applications.
Whenever necessary, it is sufficient to tell the user that the objects/memory
returned by libpq functions is allocated on the heap. That is just enough
detail for the user to realize that the relevant object/memory needs to be
freed; and the instructions that follow mention to use PQfreemem() to free such
memory.
One mention of malloc is left intact, because that mention is unrelated to how
the memory is allocated, or how to free it.
In passing, slightly improve the language of PQencryptPasswordConn()
documentation.
---
doc/src/sgml/libpq.sgml | 24 ++++++++++++------------
src/interfaces/libpq/fe-auth.c | 2 +-
2 files changed, 13 insertions(+), 13 deletions(-)
diff --git a/doc/src/sgml/libpq.sgml b/doc/src/sgml/libpq.sgml
index a52baa27d5..7da2a1d482 100644
--- a/doc/src/sgml/libpq.sgml
+++ b/doc/src/sgml/libpq.sgml
@@ -591,25 +591,25 @@ PQconninfoOption *PQconninfoParse(const char *conninfo, char **errmsg);
with an entry having a null <structfield>keyword</structfield> pointer.
</para>
<para>
All legal options will be present in the result array, but the
<literal>PQconninfoOption</literal> for any option not present
in the connection string will have <literal>val</literal> set to
<literal>NULL</literal>; default values are not inserted.
</para>
<para>
If <literal>errmsg</literal> is not <symbol>NULL</symbol>, then <literal>*errmsg</literal> is set
- to <symbol>NULL</symbol> on success, else to a <function>malloc</function>'d error string explaining
+ to <symbol>NULL</symbol> on success, else to an error string allocated on the heap, explaining
the problem. (It is also possible for <literal>*errmsg</literal> to be
set to <symbol>NULL</symbol> and the function to return <symbol>NULL</symbol>;
this indicates an out-of-memory condition.)
</para>
<para>
After processing the options array, free it by passing it to
<xref linkend="libpq-PQconninfoFree"/>. If this is not done, some memory
is leaked for each call to <xref linkend="libpq-PQconninfoParse"/>.
Conversely, if an error occurs and <literal>errmsg</literal> is not <symbol>NULL</symbol>,
be sure to free the error string using <xref linkend="libpq-PQfreemem"/>.
</para>
@@ -4543,26 +4543,26 @@ char *PQescapeLiteral(PGconn *conn, const char *str, size_t length);
<para>
<xref linkend="libpq-PQescapeLiteral"/> escapes a string for
use within an SQL command. This is useful when inserting data
values as literal constants in SQL commands. Certain characters
(such as quotes and backslashes) must be escaped to prevent them
from being interpreted specially by the SQL parser.
<xref linkend="libpq-PQescapeLiteral"/> performs this operation.
</para>
<para>
<xref linkend="libpq-PQescapeLiteral"/> returns an escaped version of the
- <parameter>str</parameter> parameter in memory allocated with
- <function>malloc()</function>. This memory should be freed using
+ <parameter>str</parameter> parameter in memory allocated on the heap.
+ This memory should be freed using
<function>PQfreemem()</function> when the result is no longer needed.
A terminating zero byte is not required, and should not be
counted in <parameter>length</parameter>. (If a terminating zero byte is found
before <parameter>length</parameter> bytes are processed,
<xref linkend="libpq-PQescapeLiteral"/> stops at the zero; the behavior is
thus rather like <function>strncpy</function>.) The
return string has all special characters replaced so that they can
be properly processed by the <productname>PostgreSQL</productname>
string literal parser. A terminating zero byte is also added. The
single quotes that must surround <productname>PostgreSQL</productname>
string literals are included in the result string.
</para>
@@ -4603,25 +4603,25 @@ char *PQescapeIdentifier(PGconn *conn, const char *str, size_t length);
<para>
<xref linkend="libpq-PQescapeIdentifier"/> escapes a string for
use as an SQL identifier, such as a table, column, or function name.
This is useful when a user-supplied identifier might contain
special characters that would otherwise not be interpreted as part
of the identifier by the SQL parser, or when the identifier might
contain upper case characters whose case should be preserved.
</para>
<para>
<xref linkend="libpq-PQescapeIdentifier"/> returns a version of the
<parameter>str</parameter> parameter escaped as an SQL identifier
- in memory allocated with <function>malloc()</function>. This memory must be
+ in memory allocated on the heap. This memory must be
freed using <function>PQfreemem()</function> when the result is no longer
needed. A terminating zero byte is not required, and should not be
counted in <parameter>length</parameter>. (If a terminating zero byte is found
before <parameter>length</parameter> bytes are processed,
<xref linkend="libpq-PQescapeIdentifier"/> stops at the zero; the behavior is
thus rather like <function>strncpy</function>.) The
return string has all special characters replaced so that it
will be properly processed as an SQL identifier. A terminating zero byte
is also added. The return string will also be surrounded by double
quotes.
</para>
@@ -4752,25 +4752,25 @@ unsigned char *PQescapeByteaConn(PGconn *conn,
byte of the string that is to be escaped, and the
<parameter>from_length</parameter> parameter gives the number of
bytes in this binary string. (A terminating zero byte is
neither necessary nor counted.) The <parameter>to_length</parameter>
parameter points to a variable that will hold the resultant
escaped string length. This result string length includes the terminating
zero byte of the result.
</para>
<para>
<xref linkend="libpq-PQescapeByteaConn"/> returns an escaped version of the
<parameter>from</parameter> parameter binary string in memory
- allocated with <function>malloc()</function>. This memory should be freed using
+ allocated on the heap. This memory should be freed using
<function>PQfreemem()</function> when the result is no longer needed. The
return string has all special characters replaced so that they can
be properly processed by the <productname>PostgreSQL</productname>
string literal parser, and the <type>bytea</type> input function. A
terminating zero byte is also added. The single quotes that must
surround <productname>PostgreSQL</productname> string literals are
not part of the result string.
</para>
<para>
On error, a null pointer is returned, and a suitable error message
is stored in the <parameter>conn</parameter> object. Currently, the only
@@ -4818,26 +4818,26 @@ unsigned char *PQescapeBytea(const unsigned char *from,
but not when retrieving it in binary format.
<synopsis>
unsigned char *PQunescapeBytea(const unsigned char *from, size_t *to_length);
</synopsis>
</para>
<para>
The <parameter>from</parameter> parameter points to a string
such as might be returned by <xref linkend="libpq-PQgetvalue"/> when applied
to a <type>bytea</type> column. <xref linkend="libpq-PQunescapeBytea"/>
converts this string representation into its binary representation.
- It returns a pointer to a buffer allocated with
- <function>malloc()</function>, or <symbol>NULL</symbol> on error, and puts the size of
+ It returns a pointer to a buffer allocated on the heap,
+ or <symbol>NULL</symbol> on error, and puts the size of
the buffer in <parameter>to_length</parameter>. The result must be
freed using <xref linkend="libpq-PQfreemem"/> when it is no longer needed.
</para>
<para>
This conversion is not exactly the inverse of
<xref linkend="libpq-PQescapeBytea"/>, because the string is not expected
to be <quote>escaped</quote> when received from <xref linkend="libpq-PQgetvalue"/>.
In particular this means there is no need for string quoting considerations,
and so no need for a <structname>PGconn</structname> parameter.
</para>
</listitem>
@@ -7086,35 +7086,35 @@ char *PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user,
<para>
The <parameter>passwd</parameter> and <parameter>user</parameter> arguments
are the cleartext password, and the SQL name of the user it is for.
<parameter>algorithm</parameter> specifies the encryption algorithm
to use to encrypt the password. Currently supported algorithms are
<literal>md5</literal> and <literal>scram-sha-256</literal> (<literal>on</literal> and
<literal>off</literal> are also accepted as aliases for <literal>md5</literal>, for
compatibility with older server versions). Note that support for
<literal>scram-sha-256</literal> was introduced in <productname>PostgreSQL</productname>
version 10, and will not work correctly with older server versions. If
<parameter>algorithm</parameter> is <symbol>NULL</symbol>, this function will query
the server for the current value of the
- <xref linkend="guc-password-encryption"/> setting. That can block, and
+ <xref linkend="guc-password-encryption"/> setting. This can block the calling thread, and
will fail if the current transaction is aborted, or if the connection
- is busy executing another query. If you wish to use the default
+ is busy executing another command. If you wish to use the default
algorithm for the server but want to avoid blocking, query
<varname>password_encryption</varname> yourself before calling
<xref linkend="libpq-PQencryptPasswordConn"/>, and pass that value as the
<parameter>algorithm</parameter>.
</para>
<para>
- The return value is a string allocated by <function>malloc</function>.
+ The return value is a string allocated on the heap.
The caller can assume the string doesn't contain any special characters
that would require escaping. Use <xref linkend="libpq-PQfreemem"/> to free the
result when done with it. On error, returns <symbol>NULL</symbol>, and
a suitable message is stored in the connection object.
</para>
</listitem>
</varlistentry>
<varlistentry id="libpq-PQencryptPassword">
<term><function>PQencryptPassword</function><indexterm><primary>PQencryptPassword</primary></indexterm></term>
@@ -7312,27 +7312,27 @@ void *PQresultAlloc(PGresult *res, size_t nBytes);
<term><function>PQresultMemorySize</function><indexterm><primary>PQresultMemorySize</primary></indexterm></term>
<listitem>
<para>
Retrieves the number of bytes allocated for
a <structname>PGresult</structname> object.
<synopsis>
size_t PQresultMemorySize(const PGresult *res);
</synopsis>
</para>
<para>
- This value is the sum of all <function>malloc</function> requests
+ This value is the sum of sizes of all memory allocation requests
associated with the <structname>PGresult</structname> object, that is,
- all the space that will be freed by <xref linkend="libpq-PQclear"/>.
+ all the memory that will be freed by <xref linkend="libpq-PQclear"/>.
This information can be useful for managing memory consumption.
</para>
</listitem>
</varlistentry>
<varlistentry id="libpq-PQlibVersion">
<term><function>PQlibVersion</function><indexterm
><primary>PQlibVersion</primary><seealso>PQserverVersion</seealso></indexterm></term>
<listitem>
<para>
Return the version of <productname>libpq</productname> that is being used.
diff --git a/src/interfaces/libpq/fe-auth.c b/src/interfaces/libpq/fe-auth.c
index 912aa14821..5a1b3795d5 100644
--- a/src/interfaces/libpq/fe-auth.c
+++ b/src/interfaces/libpq/fe-auth.c
@@ -1257,25 +1257,25 @@ PQencryptPassword(const char *passwd, const char *user)
* be sent in cleartext if it is encrypted on the client side. This is
* good because it ensures the cleartext password won't end up in logs,
* pg_stat displays, etc. We export the function so that clients won't
* be dependent on low-level details like whether the encryption is MD5
* or something else.
*
* Arguments are a connection object, the cleartext password, the SQL
* name of the user it is for, and a string indicating the algorithm to
* use for encrypting the password. If algorithm is NULL, this queries
* the server for the current 'password_encryption' value. If you wish
* to avoid that, e.g. to avoid blocking, you can execute
* 'show password_encryption' yourself before calling this function, and
- * pass it as the algorithm.
+ * pass the result as the algorithm.
*
* Return value is a malloc'd string. The client may assume the string
* doesn't contain any special characters that would require escaping.
* On error, an error message is stored in the connection object, and
* returns NULL.
*/
char *
PQencryptPasswordConn(PGconn *conn, const char *passwd, const char *user,
const char *algorithm)
{
#define MAX_ALGORITHM_NAME_LEN 50
char algobuf[MAX_ALGORITHM_NAME_LEN + 1];
--
2.41.0
On 24 Oct 2023, at 07:13, Gurjeet Singh <gurjeet@singh.im> wrote:
The user does not benefit from knowing that libpq allocates some/all memory
using malloc(). Mentioning malloc() here has a few downsides, and almost no
benefits.
I'm not entirely convinced that replacing "malloc" with "allocated on the heap"
improves the documentation. I do agree with this proposed change though:
- all the space that will be freed by <xref linkend="libpq-PQclear"/>.
+ all the memory that will be freed by <xref linkend="libpq-PQclear"/>.
--
Daniel Gustafsson
Daniel Gustafsson <daniel@yesql.se> writes:
On 24 Oct 2023, at 07:13, Gurjeet Singh <gurjeet@singh.im> wrote:
The user does not benefit from knowing that libpq allocates some/all memory
using malloc(). Mentioning malloc() here has a few downsides, and almost no
benefits.
I'm not entirely convinced that replacing "malloc" with "allocated on the heap"
improves the documentation.
That was my reaction too. The underlying storage allocator *is* malloc,
and C programmers know what that is, and I don't see how obfuscating
that improves matters. It's true that on the miserable excuse for a
platform that is Windows, you have to use PQfreemem because of
Microsoft's inability to supply a standards-compliant implementation
of malloc. But I'm not inclined to let that tail wag the dog.
I do agree with this proposed change though:
- all the space that will be freed by <xref linkend="libpq-PQclear"/>. + all the memory that will be freed by <xref linkend="libpq-PQclear"/>.
+1, seems harmless.
regards, tom lane
On 24 Oct 2023, at 17:07, Tom Lane <tgl@sss.pgh.pa.us> wrote:
Daniel Gustafsson <daniel@yesql.se> writes:
I do agree with this proposed change though:
- all the space that will be freed by <xref linkend="libpq-PQclear"/>. + all the memory that will be freed by <xref linkend="libpq-PQclear"/>.+1, seems harmless.
I've pushed this part, skipping the rest.
--
Daniel Gustafsson