Re: SSL information view

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 be null-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