Skip to content

Ameba: Mismatching allocation and deallocation #4853

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.

Sign up for GitHub

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

Closed
wants to merge 1 commit into from
Closed

Ameba: Mismatching allocation and deallocation #4853

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion targets/TARGET_Realtek/TARGET_AMEBA/RTWInterface.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -75,7 +75,7 @@ static rtw_result_t scan_result_handler( rtw_scan_handler_result_t* malloced_sca
ap.channel = record->channel;
WiFiAccessPoint *accesspoint = new WiFiAccessPoint(ap);
memcpy(&scan_handler->ap_details[scan_handler->ap_num], accesspoint, sizeof(WiFiAccessPoint));
delete[] accesspoint;
delete accesspoint;
}
scan_handler->ap_num++;
Copy link
Contributor

Choose a reason for hiding this comment

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

@Archcady Looking at the code around, got few questions, why this allocation is needed at first place, why ap_num is incremented also if the condition above is false (thus no memcpy is done) ? That memcpy is done on non POD data , should not.

@pan- Thanks for pointers

Copy link
Contributor

@Archcady Archcady Aug 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Hi @0xc0170 I think in this part we use allocation is to borrow the data structure of this class so we might use this scan_handler->ap_details[] somewhere else as a pointer to an instance of WiFiAccessPoint.
line 80 scan_handler->ap_num__; seems like being placed here on purpose, scan_handler->ap_details[scan_handler->ap_num] has reserved its place for accesspoint data, so if the condition above is false, the space is left zero and ap_num++ to start process next accesspoint data.
Thanks.

Copy link
Member

Choose a reason for hiding this comment

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

@Archcady Better use placement new then. The behavior of the program is undefined (from the C++ standard standpoint) if a non POD object is initialized with memcpy.

You can replace:

WiFiAccessPoint *accesspoint = new WiFiAccessPoint(ap);
memcpy(&scan_handler->ap_details[scan_handler->ap_num], accesspoint, sizeof(WiFiAccessPoint));
delete accesspoint;

by

new(&scan_handler->ap_details[scan_handler->ap_num]) WiFiAccessPoint(ap);

Copy link
Contributor

Choose a reason for hiding this comment

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

Thanks @pan- , BTW do I need to call destructor explicitly here? I'm not familiar with placement new in C++.

Copy link
Member

@pan- pan- Aug 4, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

@Archcady Placement new does not allocate memory from the heap. It use the address supplied in the placement param to construct the object.

In this specific case, the object will be constructed at the address &scan_handler->ap_details[scan_handler->ap_num]. You'll have to call directly the destructor when the ap_details memory block goes away or the delete[] operator if ap_details have been allocated with new[] (which I doubt since the data structure is defined locally).

} else{
Expand Down