Re: [PATCH] Expose port->authn_id to extensions and triggers

Hi,

On 8/16/22 6:58 PM, Jacob Champion wrote:
> Sounds good. v3, attached, should make the requested changes:
> - declare `struct ClientConnectionInfo`
> - use an intermediate serialization struct
> - switch to length-"prefixing" for the string
>
> I do like the way this reads compared to before.

Thanks for the new version!

+       /* Copy authn_id into the space after the struct. */
+       if (serialized.authn_id_len >= 0)

Maybe remove the "." at the end of the comment? (to be consistent with 
the other comment just above)

+/*
+ * Restore MyClientConnectionInfo from its serialized representation.
+ */
+void
+RestoreClientConnectionInfo(char *conninfo)
+{
+       SerializedClientConnectionInfo serialized;
+       char       *authn_id;

Move "char       *authn_id;" in the "if (serialized.authn_id_len >= 0)" 
below?

+
+       memcpy(&serialized, conninfo, sizeof(serialized));
+       authn_id = conninfo + sizeof(serialized);

Move "authn_id = conninfo + sizeof(serialized)" in the "if 
(serialized.authn_id_len >= 0)" below?

+
+       /* Copy the fields back into place. */

Remove the "." at the end of the comment?

+       MyClientConnectionInfo.authn_id = NULL;
+       MyClientConnectionInfo.auth_method = serialized.auth_method;
+
+       if (serialized.authn_id_len >= 0)
+               MyClientConnectionInfo.authn_id = 
MemoryContextStrdup(TopMemoryContext,
+ authn_id);

This instead?

     if (serialized.authn_id_len >= 0)
     {
         char       *authn_id;
         authn_id = conninfo + sizeof(serialized);
         MyClientConnectionInfo.authn_id = 
MemoryContextStrdup(TopMemoryContext,
authn_id);
     }

+ src/backend/utils/init/miscinit.c:RestoreClientConnectionInfo(char 
*conninfo)
+ src/include/miscadmin.h:extern void RestoreClientConnectionInfo(char 
*procinfo);

conninfo in both to be consistent?

Apart from the comments above, that looks good to me.

Regards,

-- 
Bertrand Drouvot
Amazon Web Services: https://aws.amazon.com