Comment 4 for bug 1983100

Hi Nicolas,

Thank you for the patches! I've taken a quick look at the contents, in hopes of giving you some more feedback for the v2 (in addition to Mauricio's already suggested changes).

Going through the list, I noticed that there are quite a few patches for a single bug fix, and that some of them have a substantial diffstat. There's an impression that these patches are more targeted towards including new features rather than fixing specific bugs, given their complexity and level of
changes. From the description, I understand most of these changes are probably required in order to fix faulty behavior, so we should try to get more information about each patch and what they intend to fix.

Without further context, it's not straightforward to understand why some of the patches are necessary. As an example, patch 6/7 seems to introduce a new option related to memory pinning, and patch 7/7 seems to be a Windows-specific change. It would be very helpful if you could describe your reasoning for each patch in more detail (e.g. with the "[Other Info]" section of the SRU Bug
Template), something like below:
- patch 0001 is a hard dependency of OPENSSL_INIT_NO_ATEXIT
- patch 0002 implements OPENSSL_INIT_NO_ATEXIT
- patch 0003-0004 add OPENSSL_INIT_NO_ATEXIT to the openssl test suite
...
This would make it easier to understand why each patch included, and help deciding on whether they would be appropriate for the SRU scenario.

In addition to that, it would be great to have more concrete examples in the "[Where problems could occur]" section. You mentioned that the patches could lead to incorrect behavior, so having examples of that would be helpful in spotting them once the changes go through. For instance, are these problems expected to show up in openssl itself? Would an external library or program
using openssl start exhibiting different behavior or crashes due to these changes?

I don't want to come off as excessive, but given that openssl is a somewhat 'hot' package with potentially big impact on systems, having good justification and proper context on the patches would be very beneficial when evaluating the changes.

Thanks again for the help on fixing this bug, and let us know if you have any questions!