Hi Elliot, On Wed, Jan 15, 2020 at 01:24:33PM +0000, Elliot Otchet wrote: > Willy, > The patch attached implements as close to your intent as I could get without > making changes outside of ssl_sock.c. > If you remember, you had asked for a patch that implemented extending the > current methods with a third option to indicate the DN output should be in > RFC 2253 e.g. "ssl_c_i_dn(,,rfc2253)". > This patch comes close to that. It extends the existing methods, but the > options have to be passed as "ssl_c_i_dn(,0,rfc2253)" This is because the > second argument is an integer and the arg parser doesn't like missing > integers. > > The patch includes examples in the configuration documentation that > demonstrated how to configure it for the needed output. > As always, feedback is greatly appreciated. If acceptable, what are the next > steps to get this accepted?
This looks pretty good. I'm only seeing a very minor nit which is trivial to address: the whole config is case sensitive, so better use strcmp rather than strcasecmp to match the "rfc2253" value in the 3rd argument. Otherwise there's a risk that a few users will accidently mistype it, it will work and later when passing their config through automated tools it will fail. Regarding what's missing, I'd say a commit message :-) What you described above and/or in your first presentation is OK. Please have a quick look at CONTRIBUTING and just run "git log" src/ssl_sock.c to get some inspiration on the expected format but we don't need anything complicated, just something which allows one not to have to read the patch when running "git log". Ah, be careful, you had a reindentation problem in smp_fetch_ssl_x_s_dn(), some tabs were mistakenly replaced with spaces, causing more changes than needed. Thanks! Willy
