Skip to content

Fix for double free in KdTreeFLANN after copy operation #335. #618

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

Merged
merged 1 commit into from
Apr 17, 2014

Conversation

jpajarin
Copy link
Contributor

@jpajarin jpajarin commented Apr 7, 2014

This fix changes the float *cloud_ pointer into a boost::shared_array structure, and substitutes malloc/free with new[]/delete[] operators.

@taketwo
Copy link
Member

taketwo commented Apr 9, 2014

Looks good now, though we'll need to add spaces in between function names and braces to comply to the PCL style guide. Could you do this? Then I will merge.

By the way, there is no need to close this pull request and open a new one. Simply add a new commit with a fix, then squash it with the previous one and push to your fork with --force flag. The pull request will be updated accordingly. Please let me know if you need assistance with that.

@jpajarin
Copy link
Contributor Author

Thanks for the information, this is my first bug fix on github.
I added the missing spaces. Sorry for not reading the style guide more carefully before.

I also did a git squash and push. For some reason also a commit from Nizar Sallem, which was already merged into master, is shown above (git rebase did something to the commit log?). If this is a problem, I can try to fix it.

@taketwo
Copy link
Member

taketwo commented Apr 10, 2014

For some reason also a commit from Nizar Sallem, which was already merged into master, is shown above (git rebase did something to the commit log?). If this is a problem, I can try to fix it.

The thing is, though this patch of Nizar is already in the master, it is known under a different "name" there. Specifically, it's hash is 346e44c, whereas the very same changeset in your branch goes with hash 23ad599. Therefore if I merge your request, this patch will appear twice in the history.

You can easily remove the commit though. Just do: git rebase -i HEAD~2, then delete the line with the commit in question, save and close the editor. Push with force again.

@taketwo
Copy link
Member

taketwo commented Apr 17, 2014

Okay, merging this. Thanks for the contribution, Joni!

taketwo added a commit that referenced this pull request Apr 17, 2014
Fix for double free in KdTreeFLANN after copy operation (closes #335)
@taketwo taketwo merged commit bdb91a3 into PointCloudLibrary:master Apr 17, 2014
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants