[PATCH 07 of 11] SSL: style
Maxim Dounin
mdounin at mdounin.ru
Fri Sep 16 21:05:18 UTC 2022
Hello!
On Thu, Sep 15, 2022 at 09:42:01AM +0400, Sergey Kandaurov wrote:
> > On 26 Aug 2022, at 07:01, Maxim Dounin <mdounin at mdounin.ru> wrote:
> >
> > # HG changeset patch
> > # User Maxim Dounin <mdounin at mdounin.ru>
> > # Date 1661481953 -10800
> > # Fri Aug 26 05:45:53 2022 +0300
> > # Node ID 84919c2ee8173f704649a8cb4901887e1bf79588
> > # Parent d5c6eae914325fb6a9b19105fe09aecd04da21e2
> > SSL: style.
> >
> > Runtime OCSP functions separated from configuration ones.
> >
> > diff --git a/src/event/ngx_event_openssl.h b/src/event/ngx_event_openssl.h
> > --- a/src/event/ngx_event_openssl.h
> > +++ b/src/event/ngx_event_openssl.h
> > @@ -205,10 +205,12 @@ ngx_int_t ngx_ssl_ocsp(ngx_conf_t *cf, n
> > ngx_uint_t depth, ngx_shm_zone_t *shm_zone);
> > ngx_int_t ngx_ssl_ocsp_resolver(ngx_conf_t *cf, ngx_ssl_t *ssl,
> > ngx_resolver_t *resolver, ngx_msec_t resolver_timeout);
> > +
> > ngx_int_t ngx_ssl_ocsp_validate(ngx_connection_t *c);
> > ngx_int_t ngx_ssl_ocsp_get_status(ngx_connection_t *c, const char **s);
> > void ngx_ssl_ocsp_cleanup(ngx_connection_t *c);
> > ngx_int_t ngx_ssl_ocsp_cache_init(ngx_shm_zone_t *shm_zone, void *data);
> > +
> > ngx_array_t *ngx_ssl_read_password_file(ngx_conf_t *cf, ngx_str_t *file);
> > ngx_array_t *ngx_ssl_preserve_passwords(ngx_conf_t *cf,
> > ngx_array_t *passwords);
> >
>
> Speaking of style, this reminds me of various more style issues.
There was no goal to fix all the style issues. This particular
one interfered with ngx_event_openssl.h changes, hence it was
fixed.
Note well that a generic rule is to avoid style changes without a
good reason.
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1663066823 -14400
> # Tue Sep 13 15:00:23 2022 +0400
> # Node ID e3da137555cfb6a3eb80aae196a49b945a4f5048
> # Parent 3b0846bd090e06cf277879d4ba4a67a0a2569233
> SSL: style.
>
> Using suitable naming for SSL_CTX variables.
I would rather say that ssl_ctx isn't, but in most cases we have
little to no choice.
[...]
> # HG changeset patch
> # User Sergey Kandaurov <pluknet at nginx.com>
> # Date 1663199989 -14400
> # Thu Sep 15 03:59:49 2022 +0400
> # Node ID b13b26ab24e9f12a808301bf4c8713d52c7944aa
> # Parent e3da137555cfb6a3eb80aae196a49b945a4f5048
> SSL: fixed indentation.
>
> diff --git a/src/event/ngx_event_openssl.c b/src/event/ngx_event_openssl.c
> --- a/src/event/ngx_event_openssl.c
> +++ b/src/event/ngx_event_openssl.c
> @@ -998,12 +998,12 @@ static int
> ngx_ssl_verify_callback(int ok, X509_STORE_CTX *x509_store)
> {
> #if (NGX_DEBUG)
> + int err, depth;
> char *subject, *issuer;
> - int err, depth;
> X509 *cert;
> X509_NAME *sname, *iname;
> + ngx_ssl_conn_t *ssl_conn;
> ngx_connection_t *c;
> - ngx_ssl_conn_t *ssl_conn;
>
> ssl_conn = X509_STORE_CTX_get_ex_data(x509_store,
> SSL_get_ex_data_X509_STORE_CTX_idx());
> @@ -2274,8 +2274,8 @@ ngx_ssl_recv(ngx_connection_t *c, u_char
> static ssize_t
> ngx_ssl_recv_early(ngx_connection_t *c, u_char *buf, size_t size)
> {
> - int n, bytes;
> - size_t readbytes;
> + int n, bytes;
> + size_t readbytes;
>
> if (c->ssl->last == NGX_ERROR) {
> c->read->error = 1;
> @@ -2528,9 +2528,9 @@ ngx_chain_t *
> ngx_ssl_send_chain(ngx_connection_t *c, ngx_chain_t *in, off_t limit)
> {
> int n;
> - ngx_uint_t flush;
> ssize_t send, size, file_size;
> ngx_buf_t *buf;
> + ngx_uint_t flush;
> ngx_chain_t *cl;
>
> if (!c->ssl->buffer) {
> @@ -3491,9 +3491,9 @@ ngx_ssl_error(ngx_uint_t level, ngx_log_
> {
> int flags;
> u_long n;
> - va_list args;
> u_char *p, *last;
> u_char errstr[NGX_MAX_CONF_ERRSTR];
> + va_list args;
> const char *data;
>
> last = errstr + NGX_MAX_CONF_ERRSTR;
> @@ -3809,12 +3809,12 @@ ngx_ssl_new_session(ngx_ssl_conn_t *ssl_
> int len;
> u_char *p, *session_id;
> size_t n;
> + SSL_CTX *ssl_ctx;
> uint32_t hash;
> - SSL_CTX *ssl_ctx;
> unsigned int session_id_length;
> ngx_shm_zone_t *shm_zone;
> + ngx_slab_pool_t *shpool;
> ngx_connection_t *c;
> - ngx_slab_pool_t *shpool;
> ngx_ssl_sess_id_t *sess_id;
> ngx_ssl_session_cache_t *cache;
>
> @@ -3959,12 +3959,12 @@ ngx_ssl_get_cached_session(ngx_ssl_conn_
> const u_char *p;
> ngx_shm_zone_t *shm_zone;
> ngx_slab_pool_t *shpool;
> + ngx_connection_t *c;
> ngx_rbtree_node_t *node, *sentinel;
> ngx_ssl_session_t *sess;
> ngx_ssl_sess_id_t *sess_id;
> ngx_ssl_session_cache_t *cache;
> u_char buf[NGX_SSL_MAX_SESSION_SIZE];
> - ngx_connection_t *c;
>
> hash = ngx_crc32_short((u_char *) (uintptr_t) id, (size_t) len);
> *copy = 0;
> @@ -4500,7 +4500,7 @@ ngx_ssl_session_ticket_key_callback(ngx_
> static void
> ngx_ssl_session_ticket_keys_cleanup(void *data)
> {
> - ngx_array_t *keys = data;
> + ngx_array_t *keys = data;
Last time I've checked, there were no clear rule to use just one
space in such cases. Rather, one space is allowed and may be
preferred, but certainly not required.
$ grep -re '^ [0-9a-z_]\+ [0-9a-z_*]\+ = ' src/ | wc -l
169
$ grep -re '^ [0-9a-z_]\+ [0-9a-z_*]\+ = ' src/ | wc -l
257
>
> ngx_explicit_memzero(keys->elts,
> keys->nelts * sizeof(ngx_ssl_session_ticket_key_t));
> @@ -4525,7 +4525,7 @@ ngx_ssl_session_ticket_keys(ngx_conf_t *
> void
> ngx_ssl_cleanup_ctx(void *data)
> {
> - ngx_ssl_t *ssl = data;
> + ngx_ssl_t *ssl = data;
>
> X509 *cert, *next;
>
> @@ -4544,7 +4544,7 @@ ngx_ssl_cleanup_ctx(void *data)
> ngx_int_t
> ngx_ssl_check_host(ngx_connection_t *c, ngx_str_t *name)
> {
> - X509 *cert;
> + X509 *cert;
>
> cert = SSL_get_peer_certificate(c->ssl->connection);
> if (cert == NULL) {
> @@ -4575,8 +4575,8 @@ ngx_ssl_check_host(ngx_connection_t *c,
> int n, i;
> X509_NAME *sname;
> ASN1_STRING *str;
> + GENERAL_NAME *altname;
> X509_NAME_ENTRY *entry;
> - GENERAL_NAME *altname;
> STACK_OF(GENERAL_NAME) *altnames;
>
> /*
Here GENERAL_NAME and STACK_OF(GENERAL_NAME) are intentionally
defined close to each other.
> @@ -4851,9 +4851,9 @@ ngx_ssl_get_curves(ngx_connection_t *c,
> {
> #ifdef SSL_CTRL_GET_CURVES
>
> - int *curves, n, i, nid;
> - u_char *p;
> - size_t len;
> + int *curves, n, i, nid;
> + u_char *p;
> + size_t len;
>
> n = SSL_get1_curves(c->ssl->connection, NULL);
>
> @@ -5046,9 +5046,9 @@ ngx_ssl_get_alpn_protocol(ngx_connection
> ngx_int_t
> ngx_ssl_get_raw_certificate(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
> {
> - size_t len;
> BIO *bio;
> X509 *cert;
> + size_t len;
>
> s->len = 0;
>
> @@ -5098,8 +5098,8 @@ ngx_ssl_get_certificate(ngx_connection_t
> {
> u_char *p;
> size_t len;
> + ngx_str_t cert;
> ngx_uint_t i;
> - ngx_str_t cert;
>
> if (ngx_ssl_get_raw_certificate(c, pool, &cert) != NGX_OK) {
> return NGX_ERROR;
> @@ -5280,8 +5280,8 @@ ngx_ssl_get_subject_dn_legacy(ngx_connec
> ngx_str_t *s)
> {
> char *p;
> + X509 *cert;
> size_t len;
> - X509 *cert;
> X509_NAME *name;
>
> s->len = 0;
> @@ -5328,8 +5328,8 @@ ngx_ssl_get_issuer_dn_legacy(ngx_connect
> ngx_str_t *s)
> {
> char *p;
> + X509 *cert;
> size_t len;
> - X509 *cert;
> X509_NAME *name;
>
> s->len = 0;
> @@ -5374,9 +5374,9 @@ ngx_ssl_get_issuer_dn_legacy(ngx_connect
> ngx_int_t
> ngx_ssl_get_serial_number(ngx_connection_t *c, ngx_pool_t *pool, ngx_str_t *s)
> {
> - size_t len;
> + BIO *bio;
> X509 *cert;
> - BIO *bio;
> + size_t len;
>
> s->len = 0;
>
In many SSL functions, including these, generic types and SSL
types are defined separately. This is a clear style pattern, not
a style issue.
Overall, I would rather not.
--
Maxim Dounin
http://mdounin.ru/
More information about the nginx-devel
mailing list