Re: SSL information view - Mailing list pgsql-hackers

On 11/19/2014 02:36 PM, Magnus Hagander wrote:
> +     /* Create or attach to the shared SSL status buffers */
> +     size = mul_size(NAMEDATALEN, MaxBackends);
> +     BackendSslVersionBuffer = (char *)
> +         ShmemInitStruct("Backend SSL Version Buffer", size, &found);
> +
> +     if (!found)
> +     {
> +         MemSet(BackendSslVersionBuffer, 0, size);
> +
> +         /* Initialize st_ssl_version pointers. */
> +         buffer = BackendSslVersionBuffer;
> +         for (i = 0; i < MaxBackends; i++)
> +         {
> +             BackendStatusArray[i].st_ssl_version = buffer;
> +             buffer += NAMEDATALEN;
> +         }
> +     }
> +
> +     size = mul_size(NAMEDATALEN, MaxBackends);
> +     BackendSslCipherBuffer = (char *)
> +         ShmemInitStruct("Backend SSL Cipher Buffer", size, &found);
> +
> +     if (!found)
> +     {
> +         MemSet(BackendSslCipherBuffer, 0, size);
> +
> +         /* Initialize st_ssl_cipher pointers. */
> +         buffer = BackendSslCipherBuffer;
> +         for (i = 0; i < MaxBackends; i++)
> +         {
> +             BackendStatusArray[i].st_ssl_cipher = buffer;
> +             buffer += NAMEDATALEN;
> +         }
> +     }
> +
> +     size = mul_size(NAMEDATALEN, MaxBackends);
> +     BackendSslClientDNBuffer = (char *)
> +         ShmemInitStruct("Backend SSL Client DN Buffer", size, &found);
> +
> +     if (!found)
> +     {
> +         MemSet(BackendSslClientDNBuffer, 0, size);
> +
> +         /* Initialize st_ssl_clientdn pointers. */
> +         buffer = BackendSslClientDNBuffer;
> +         for (i = 0; i < MaxBackends; i++)
> +         {
> +             BackendStatusArray[i].st_ssl_clientdn = buffer;
> +             buffer += NAMEDATALEN;
> +         }
> +     }

This pattern gets a bit tedious. We do that already for 
application_names, client hostnames, and activity status but this adds 
three more such strings. Why are these not just regular char arrays in 
PgBackendStatus struct, anyway? The activity status is not, because its 
size is configurable with the pgstat_track_activity_query_size GUC, but 
all those other things are fixed-size.

Also, it would be nice if you didn't allocate the memory for all those 
SSL strings, when SSL is disabled altogether. Perhaps put the 
SSL-related information into a separate struct:

struct
{     /* Information about SSL connection */     int        st_ssl_bits;     bool        st_ssl_compression;     char
    st_ssl_version[NAMEDATALEN];  /* MUST be null-terminated */     char        st_ssl_cipher[NAMEDATALEN];   /* MUST
benull-terminated */     char        st_ssl_clientdn[NAMEDATALEN]; /* MUST be null-terminated */
 
} PgBackendSSLStatus;

Those structs could be allocated like you allocate the string buffers 
now, with a pointer to that struct from PgBackendStatus. When SSL is 
disabled, the structs are not allocated and the pointers in 
PgBackendStatus structs are NULL.

- Heikki