On Mon, Apr 7, 2025 at 3:19 AM Michael Paquier <michael@paquier.xyz> wrote: > > On Sun, Apr 06, 2025 at 12:37:24PM -0500, Steve Chavez wrote: > > I found the numbers in `quote_literal_cstr` palloc quite magical. So I've > > added a comment clarifying what they mean. The change is small: > > > > /* We make a worst-case result area; wasting a little space is OK */ > > - result = palloc(len * 2 + 3 + 1); > > + result = palloc( > > + (len * 2) /* worst-case doubling for every character if > > each one is a quote */ > > + + 3 /* two outer quotes + possibly 'E' if needed */ > > + + 1 /* null terminator */ > > + ); > > Agreed that this is a good idea to have a better documentation here if > someone decides to tweak this area in the future, rather than have > them guess the numbers. > > Looking at what quote_literal_internal() does, it seems to me that you > are right based on ESCAPE_STRING_SYNTAX, SQL_STR_DOUBLE(), and the > extra backslashes added to the destination buffer.
This is an improvement in readability. However, I have not seen this style of commenting arguments of a function. If there's a precedence, please let me know. Usually we explain it in a comment before the call. For example, something like /* Allocate memory for worst case result factoring in a. double the length number of bytes, in case every character is a quote, b. two bytes for two outer quotes c. extra byte for E' d. one byte for null terminator. */
