Skip to content

Conversation

@dgud
Copy link
Contributor

@dgud dgud commented Nov 6, 2025

Issues reported in the excellent blog post at
https://pvs-studio.com/en/blog/posts/cpp/1305/

@dgud dgud self-assigned this Nov 6, 2025
@dgud dgud added the team:PS Assigned to OTP team PS label Nov 6, 2025
@github-actions
Copy link
Contributor

github-actions bot commented Nov 6, 2025

CT Test Results

  2 files   17 suites   4m 24s ⏱️
158 tests 151 ✅ 7 💤 0 ❌
174 runs  167 ✅ 7 💤 0 ❌

Results for commit aa16380.

♻️ This comment has been updated with latest results.

To speed up review, make sure that you have read Contributing to Erlang/OTP and that all checks pass.

See the TESTING and DEVELOPMENT HowTo guides for details about how to run test locally.

Artifacts

// Erlang/OTP Github Action Bot

@dgud dgud requested a review from kvakvs November 6, 2025 16:34
@kvakvs
Copy link
Collaborator

kvakvs commented Nov 7, 2025

I just checked wxImage constructors (base of EwxImage) documentation and it says that in case when the 5th parameter static_data is having default value false, wx will take ownership of the data and for that it must be allocated by malloc. Such is the way they do things in the C kingdom. So i guess the change is sufficient, but brittle (till the generated code changes next time again).

(since C++ is already in this file, can use C++'s auto-memory management facilities) Alternative would be for Ewximage to own its memory: use a smart pointer to allocate and own pixels data, and then any kind of destruction operation would automatically free it. That would look beautiful but will also increase the change scope.

* dgud/wx/bugs/OTP-19843:
  wx: Fix array reading out of bounds, and potential memory leaks
@dgud dgud force-pushed the dgud/wx/bugs/28/OTP-19843 branch from ee84e20 to aa16380 Compare November 7, 2025 08:06
@dgud dgud added the testing currently being tested, tag is used by OTP internal CI label Nov 7, 2025
Copy link
Collaborator

@kvakvs kvakvs left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wholeheartedly support a bigger review of memory management patterns in generated and hand-written code in the future. Would love to join :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

team:PS Assigned to OTP team PS testing currently being tested, tag is used by OTP internal CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants