-
Notifications
You must be signed in to change notification settings - Fork 342
[All] Security : memory management, destructors and al #5904
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
base: master
Are you sure you want to change the base?
Conversation
- Issue: fwrite() returns number of items written, not bytes. Comparison was incorrect. - Fix: Changed comparisons from == sizeof(DDSHeader) and == getImageSize() to == 1
…d cause undefined behavior when deleting derived objects through base pointers
…y leak in debug file
eb25a78 to
b0871a3
Compare
|
[ci-build][with-all-tests] |
| volatile unsigned head; | ||
| volatile unsigned tail; | ||
| std::atomic<unsigned> head; | ||
| std::atomic<unsigned> tail; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not fully confident on that
| << " -crf 17 " | ||
| << " -vf vflip " | ||
| << "\"" << m_filename << "\""; // @TODO C++14 : replace with std::quoted | ||
| << "\"" << escapeForShell(m_filename) << "\""; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah ok, but the filename is something we create so this sanity check is maybe not required
|
|
||
| // delete argv copy | ||
| for (int i = 0; i < m_argc; i++) { | ||
| delete[] copyArgv[i]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Isn't this still required because we still have a vector of char* ?
(the misc last part)
[with-all-tests]
By submitting this pull request, I acknowledge that
I have read, understand, and agree SOFA Developer Certificate of Origin (DCO).
Reviewers will merge this pull-request only if