-
-
Notifications
You must be signed in to change notification settings - Fork 11.1k
Description
This is no doubt too late for 1.1.0, but perhaps 1.2.0. (Or perhaps earlier if you don't mind making some APIs start kinda lying.)
The function codes in err are kind of nuts. They make development cumbersome (function codes must be updated whenever a new function that uses err is added), are a backwards compatibility nightmare if anyone tries to condition on them (rare, but happens on occasion), easy to mess up (code routinely uses the wrong one?), and impede refactoring like renaming non-public functions.
In BoringSSL, we got rid of them. First with the following scheme:
- The error code itself never contains a function code number. It's always zero.
ERR_func_error_stringalways returns the special valueOPENSSL_internal. Imagine that OpenSSL decided to turn every function into a wrapper over a static function namedOPENSSL_internal. It'd be nonsense but legal C. This extends to be behavior ofERR_error_string_n.ERR_put_errorcurrently samples and stores__FILE__and__LINE__out-of-band. Also sample__func__and store that. This requires changingERR_put_erroror adding anERR_put_error_ex.ERR_print_errors_cbextracted this internal value and, rather than the unhelpfulOPENSSL_internal, printed the true out-of-band function name.
That was a huge productivity win, but not a binary size one. We later decided sampling __func__ really wasn't very useful and wasted binary size in every BoringSSL consumer. We undid the changes to ERR_put_error and ERR_print_errors_cb. Now we always report function names as OPENSSL_internal.
This is fairly simple and seems to have worked just fine. If you're using the function codes for logging, the file and line number are readily available and strictly more useful. If you're using them for anything programmatic... don't. No other library lets consumers condition on the internal implementation details like this.