On Mon, Jul 11, 2022 at 6:09 AM Peter Eisentraut <peter.eisentr...@enterprisedb.com> wrote: > I squashed those two together. I also adjusted the error message a bit > more for project style. (We can put both lines into detail.)
Oh, okay. Log parsers don't have any issues with that? > I had to read up on this "ex_data" API. Interesting. But I'm wondering > a bit about how the life cycle of these objects is managed. What > happens if the allocated error string is deallocated before its > containing object? Or vice versa? Yeah, I'm currently leaning heavily on the lack of any memory context switches here. And I end up leaking out a pointer to the stale stack of be_tls_open_server(), which is gross -- it works since there are no other clients, but that could probably come back to bite us. The ex_data API exposes optional callbacks for new/dup/free (I'm currently setting those to NULL), so we can run custom code whenever the SSL* is destroyed. If you'd rather the data have the same lifetime of the SSL* object, we can switch to malloc/strdup/free (or even OPENSSL_strdup() in later versions). But since we don't have any use for the ex_data outside of this function, maybe we should just clear it before we return, rather than carrying it around. > How do we ensure we don't > accidentally reuse the error string when the code runs again? (I guess > currently it can't?) The ex_data is associated with the SSL*, not the global SSL_CTX*, so that shouldn't be an issue. A new SSL* gets created at the start of be_tls_open_server(). > Maybe we should avoid this and just put the > errdetail itself into a static variable that we unset once the message > is printed? If you're worried about the lifetime of the palloc'd data being too short, does switching to a static variable help in that case? --Jacob