Skip to content
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

changed: Curl's overflow buffer handling #7198

Merged
merged 1 commit into from Jun 2, 2015

Conversation

arnova
Copy link
Member

@arnova arnova commented May 28, 2015

@mkortstiege : Please review.

@arnova arnova force-pushed the curl_memleak_fix branch 2 times, most recently from ea17dab to 445dcd5 Compare May 28, 2015 14:01
@arnova
Copy link
Member Author

arnova commented May 28, 2015

jenkins build this please

@arnova arnova force-pushed the curl_memleak_fix branch 3 times, most recently from d823341 to 5004105 Compare May 28, 2015 17:40
@arnova arnova changed the title fixed: Memory leak in Curl's overflow buffer handling changed: Curl's overflow buffer handling May 28, 2015
@arnova arnova force-pushed the curl_memleak_fix branch 2 times, most recently from 2ed2369 to 277d3dd Compare May 29, 2015 14:30
@mkortstiege
Copy link
Member

Not my department really. Maybe @Paxxi could have a look at this as well?

@Paxxi
Copy link
Member

Paxxi commented May 29, 2015

@arnova Both calls to memcpy should be replaced with memmove afaik. Memcpy where dst and src overlap is undefined behaviour, will need to go through it more thoroughly for rest of the review.

@arnova
Copy link
Member Author

arnova commented May 29, 2015

@Paxxi: Yeah but memmove is slower since it uses an extra buffer and doesn't memcpy start at the bottom?

@fritsch
Copy link
Member

fritsch commented May 29, 2015

If it overlaps -> no memcpy.

@arnova
Copy link
Member Author

arnova commented May 29, 2015

@fritsch / @Paxxi: Thanks for the heads up. Just read that memmove() may not be that slower after all anyway. Changed it to memmove now.

@arnova
Copy link
Member Author

arnova commented May 31, 2015

jenkins build this please

@MartijnKaijser
Copy link
Member

ignore build fail

@arnova
Copy link
Member Author

arnova commented Jun 2, 2015

@Paxxi : You're fine with this?

@Paxxi
Copy link
Member

Paxxi commented Jun 2, 2015

@arnova yup looks fine

arnova added a commit that referenced this pull request Jun 2, 2015
changed: Curl's overflow buffer handling
@arnova arnova merged commit d0a4bc6 into xbmc:master Jun 2, 2015
@hudokkow hudokkow added this to the Isengard 15.0-beta2 milestone Jun 2, 2015
@arnova arnova deleted the curl_memleak_fix branch January 10, 2016 10:49
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.

None yet

6 participants