ERR: re-use the err_data field when possible#9459
ERR: re-use the err_data field when possible#9459levitte wants to merge 3 commits intoopenssl:masterfrom
Conversation
|
I don't think you need a new flag. You just need to change the meaning of the "MALLOCED" flag to mean "owned by ERR facility" |
|
So how do you differentiate between an err_data that has been allocated and has content that belongs with the error record, and err_data that has been allocated but belonged to an error record that is now gone? |
To deallocate the err_data field and then allocating it again might be a waste of processing, but may also be a source of errors when memory is scarce. While we normally tolerate that, the ERR sub-system is an exception and we need to pay closer attention to how we handle memory. This adds a new err_data flag, ERR_TXT_IGNORE, which means that even if there is err_data memory allocated, its contents should be ignored. Deallocation of the err_data field is much more selective, aand should only happen when ERR_free_state() is called. Fixes openssl#9458
levitte
force-pushed
the
err-reuse-data-buffer
branch
from
401aa0b to
5d0ce3f
Compare
levitte
changed the title
|
The CIs seem happy, my personal tests are happy, so this is no longer WIP |
You keep the data pointer until completely releasing all the error-data. In order to re-use the data pointer, of course, you have to move away from free'ing it each time it comes to the top of the queue |
|
... you lost me. Do you understand what I'm asking? Basically, if |
|
Hmmm... Another way to deal with the situation is by clearing the data itself, i.e. |
|
* ... you lost me. Do you understand what I'm asking? Basically, if err_data is non-NULL, how do I know that it points at valid data for the error record or if it points at old data that's lingering from a previous record?
Oh, thanks. Yes that’s a good point, and that’s why you need something like the “re-use” flag you have.
|
|
Hey @richsalz, you might want to look at |
|
(wrong commit, corrected) |
|
The change is nice. |
|
glad you like it :-) |
paulidale
left a comment
paulidale
left a comment
There was a problem hiding this comment.
One typo to fix but good to go.
levitte
added
the
approval: done
|
Merged. 10f8b36 ERR: re-use the err_data field when possible |
To deallocate the err_data field and then allocating it again might be a waste of processing, but may also be a source of errors when memory is scarce. While we normally tolerate that, the ERR sub-system is an exception and we need to pay closer attention to how we handle memory. This adds a new err_data flag, ERR_TXT_IGNORE, which means that even if there is err_data memory allocated, its contents should be ignored. Deallocation of the err_data field is much more selective, aand should only happen when ERR_free_state() is called. Fixes #9458 Reviewed-by: Paul Dale <[email protected]> (Merged from openssl/openssl#9459)
3 participants
To deallocate the err_data field and then allocating it again might be
a waste of processing, but may also be a source of errors when memory
is scarce. While we normally tolerate that, the ERR sub-system is an
exception and we need to pay closer attention to how we handle memory.
This adds a new err_data flag, ERR_TXT_IGNORE, which means that even
if there is err_data memory allocated, its contents should be ignored.
Deallocation of the err_data field is much more selective, aand should
only happen when ERR_free_state() is called.
Fixes #9458