Hi, Lev Thanks for review, I'll make improvements in V2. -- Best Regards, Vladislav Grishenko
-----Original Message----- From: Lev Stipakov <lstipa...@gmail.com> Sent: Wednesday, August 5, 2020 1:29 PM To: Vladislav Grishenko <themi...@yandex-team.ru> Cc: openvpn-devel <openvpn-devel@lists.sourceforge.net> Subject: Re: [Openvpn-devel] [PATCH] Log serial number of revoked certificate Hi, Compiled and tested on Ubuntu 20.04, looks good. A few nit-picks: > +verify_check_crl_dir(const char *crl_dir, int cert_depth, > +openvpn_x509_cert_t *cert, char *subject) The last parameter could benefit from const to indicate that function is not going to modify it. > - msg(D_HANDSHAKE, "VERIFY CRL: certificate serial number %s is > revoked", serial); > + msg(D_HANDSHAKE, "VERIFY CRL: depth=%d, %s, serial=%s is revoked", > + cert_depth, subject, serial); Since you are modifying this line, could you add a NULL check to serial to and pass something like "<not available>" in this case? > + msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, subject=%s, > serial=%s: %s", > + cert_depth, subject, serial ? serial : "", errstr); I would use "<not available>" in NULL case, otherwise the error message becomes funny. > + msg(D_TLS_ERRORS, "VERIFY ERROR: depth=%d, error=%s: %s, > + serial=%s", > X509_STORE_CTX_get_error_depth(ctx), > X509_verify_cert_error_string(X509_STORE_CTX_get_error(ctx)), > - subject); > + subject, serial ? serial : ""); Same as above. _______________________________________________ Openvpn-devel mailing list Openvpn-devel@lists.sourceforge.net https://lists.sourceforge.net/lists/listinfo/openvpn-devel
