Hi Rémi, On Tue, Oct 20, 2015 at 10:39:16AM +0200, Remi Gacogne wrote: > Hi, > > On 10/19/2015 05:01 PM, Willy Tarreau wrote: > >> [1] https://www.mail-archive.com/haproxy@formilux.org/msg19962.html > >> [2] https://www.mail-archive.com/haproxy@formilux.org/msg19995.html > > > > Regarding the second one, maybe Rémi's review could help. I noticed that > > you used gen_ssl_ctx_ptr_index = -1 which is the same value used for > > dh_params. Based on its name it makes me think it's a position in an > > array, so I'm not sure whether we can make them collide for example. I > > really don't know this API at all. > > I am not familiar with the generated certificate cache, so I will just > comment on the use of SSL_CTX_set_ex_data() and SSL_CTX_get_ex_data() in > the second patch, at least for now.
Thank you, that was the part giving me some doubts. > The value of gen_ssl_ctx_ptr_index is correctly initialized to -1, and a > valid index is then obtained by calling SSL_CTX_get_ex_new_index() in > the __ssl_sock_init() constructor, so there should not be any collision > with other indexes. OK great then! > My only minor remark is that, though unlikely, > SSL_CTX_get_ex_new_index() might return -1 in case of error. In the DH > code we handle this by checking that ssl_dh_ptr_index is != -1 before > using it, and I think it would be better to do the same check before > using gen_ssl_ctx_ptr_index. Good catch indeed. Thanks for your insights. Best regards, Willy
