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

BUG: Prevent VOID_copyswapn ignoring strides #8976

Merged
merged 2 commits into from
Apr 22, 2017

Conversation

eric-wieser
Copy link
Member

Previously, this would cause a segfault in .sort.

Fixes #8210

Previously, this would cause a segfault in .sort.

Fixes numpy#8210
@charris
Copy link
Member

charris commented Apr 22, 2017

LGTM once the implicit declaration problem is fixed. I've often thought it would be good if we actually declared all functions at the top of the file before gettin around to the definitions.

@eric-wieser
Copy link
Member Author

eric-wieser commented Apr 22, 2017

Fixed in a different way - having these functions call each other is a recipe for bugs if we end up modifying one and accidentally affecting the other. Better just to pull out some helper functions.

I think this first commit alone should be enough to backport, if you want to not do so to MAINT commits

@eric-wieser eric-wieser added the 09 - Backport-Candidate PRs tagged should be backported label Apr 22, 2017
@@ -1796,6 +1796,30 @@ BOOL_fromstr(char *str, void *ip, char **endptr,
*/


NPY_INLINE void
Copy link
Contributor

Choose a reason for hiding this comment

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

needs static

@eric-wieser
Copy link
Member Author

@juliantaylor: Done

@juliantaylor
Copy link
Contributor

can you squash the commits?
I don't like having known broken commits in the history for bisection.

@eric-wieser
Copy link
Member Author

@juliantaylor: I'm not sure the first commit is broken? I think the tests just got cancelled

@eric-wieser
Copy link
Member Author

It looks like I managed to confuse travis though, as the later tests are definitely running with a different set of source code to the early ones

@juliantaylor
Copy link
Contributor

hm looks like it. Its fine then thanks

@juliantaylor juliantaylor merged commit 58a9900 into numpy:master Apr 22, 2017
@eric-wieser
Copy link
Member Author

eric-wieser commented Apr 22, 2017

Alarmingly, travis doesn't even have the same sha1 as the commit that cross is by

@eric-wieser eric-wieser deleted the void-copyswapn branch April 22, 2017 12:30
@charris charris added 09 - Backport-Candidate PRs tagged should be backported and removed 09 - Backport-Candidate PRs tagged should be backported labels May 13, 2017
@charris charris added this to the 1.12.2 release milestone May 13, 2017
@charris charris removed the 09 - Backport-Candidate PRs tagged should be backported label Sep 22, 2017
@charris charris removed this from the 1.12.2 release milestone Sep 22, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants