-
-
Notifications
You must be signed in to change notification settings - Fork 963
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fixed the memory burn #1185
base: master
Are you sure you want to change the base?
Fixed the memory burn #1185
Conversation
Fixed the memory clearing function a little bit
Thank you for your contribution to VeraCrypt! I've reviewed your proposed changes to the burn macro, and I have some concerns:
Could you provide more information about the motivation behind these changes? Did you notice a specific issue with the original implementation? And as previously mentioned, have you validated at the assembly level that the generated assembly code does not optimize out this logic? |
The idea of the change was to make the memory cleanup macro more autonomous and universal. Additional checks were implemented for this purpose. As for the speed, I assumed that this macro is not used for large volumes of data, so these changes would not significantly affect the speed. As for compiler optimizations, it's interesting, I will conduct several experiments with different compilation flags and provide a listing. |
Thank you for clarifying your intentions behind the changes to the burn macro. Regarding the notion of making the memory cleanup macro "more autonomous and universal", could you provide more insight into what you mean by that? Are there specific use cases or scenarios you have in mind where the current macro doesn't work as intended? Understanding your perspective will help in evaluating the need for such changes. While it's true that the burn macro might not be used for large volumes of data, performance is still a critical concern for us. Even small overheads can add up when the macro is called frequently, and I want to ensure that any changes we make do not inadvertently introduce performance bottlenecks. As for the NULL check, I understand the rationale behind avoiding access violations. Still, I believe that a more appropriate place to handle such scenarios would be elsewhere in the codebase. The burn macro should ideally be focused on its primary function - clearing memory. Adding NULL checks within the macro could potentially mask other issues that might arise due to passing NULL pointers inadvertently. Lastly, I appreciate your willingness to experiment with different compilation flags to assess the impact on compiler optimizations. Please do share your findings, as they will be crucial in determining the viability of your proposed changes. In conclusion, while I appreciate your initiative and the thought process behind your proposal, I'm still trying to understand the core issue that your changes aim to address. If you could shed more light on the specific problem you're trying to solve, it would help guide my discussion further. Looking forward to your response! |
Fixed the memory clearing function a little bit