On Fri, Sep 12, 2014 at 09:39, Kent R. Spillner wrote: > On Fri, Sep 12, 2014 at 12:38:07AM -0700, Doug Hogan wrote: >> Hmm this doesn't look right to me. ressl_config is not allocated the >> same way as the other variables. The other variables such as ssl, >> sslhost, etc are local to that function and allocated there. >> ressl_config is a global and only allocated in main.c. It is allocated >> once and configured using getopt/getsubopt(). >> >> If you free it, you would need to reallocate it each time or set it to >> NULL so libressl will revert back and discard the user's changes. >> I think either way is wrong. The SSL configuration should be retained >> even if a single url_get() fails (which isn't fatal). >> >> The above code is from url_get() which is called in a loop inside >> auto_fetch(). I think the above change would use the getsubopt() >> configuration for the first HTTPS URL argument and then try to use freed >> memory for the others. > > Of course, you're right. Is there any reason why ressl_config must be > global? It's only used in url_get. It should be harmless to create a > new one each time as long as we always free it.
If the config doesn't change, I don't think this gains us anything. The point of separating the config from the context was exactly so that you could use the same config several times. I think "leaking" the config is fine as it is.