Hi Daniel, Thanks for taking care of this patch.
> On Sep 25, 2025, at 15:46, Daniel Gustafsson <[email protected]> wrote: > >> On 23 Sep 2025, at 09:50, Chao Li <[email protected]> wrote: > >> Again, found an incorrect function comment of stringToNodeInternal(). Wrong >> function name is put to the comment, I guess it was from a copy/paste error. > > I think this is intentional, and the proposed change would not improve it. > The > comment refers to the externally visible symbols stringToNodeWithLocations > (when enabled) and stringToNode The stringToNodeInternal function is just an > implementation detail of it, hence the comment further down on the actual > stringToNode() function being "Externally visible entry points". > I searched for similar cases: This one just uses the right function name in comment: /* * CommitTransactionCommandInternal - a function doing an iteration of work * regarding handling the commit transaction command. In the case of * subtransactions more than one iterations could be required. Returns * true when no more iterations required, false otherwise. */ static bool CommitTransactionCommandInternal(void) { This one say “body of” the actual function: /* * Body of createTrgmNFA, exclusive of regex compilation/freeing. */ static TRGM * createTrgmNFAInternal(regex_t *regex, TrgmPackedGraph **graph, MemoryContext rcontext) { This one just doesn’t mention function name in the comment: /* * Split internal page and insert new data. * * Returns new temp pages to *newlpage and *newrpage. * The original buffer is left untouched. */ static void dataSplitPageInternal(GinBtree btree, Buffer origbuf, This one does the same ways as stringToNode: /* * PQescapeBytea - converts from binary string to the * minimal encoding necessary to include the string in an SQL * INSERT statement with a bytea type column as the target. * * We can use either hex or escape (traditional) encoding. * In escape mode, the following transformations are applied: * '\0' == ASCII 0 == \000 * '\'' == ASCII 39 == '' * '\\' == ASCII 92 == \\ * anything < 0x20, or > 0x7e ---> \ooo * (where ooo is an octal expression) * * If not std_strings, all backslashes sent to the output are doubled. */ static unsigned char * PQescapeByteaInternal(PGconn *conn, And this one doesn’t have a function comment: static int PQsendQueryInternal(PGconn *conn, const char *query, bool newQuery) There are 5 different cases, showing that there is not a unique way for what function name should be put to xxInternal() functions’ comment. Is it deserve to take this opportunity to make all of them in a consistent format? Best regards, -- Chao Li (Evan) HighGo Software Co., Ltd. https://www.highgo.com/
