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: fix uninitialized use of size 1 reductions #4535

Merged
merged 1 commit into from
Mar 23, 2014

Conversation

juliantaylor
Copy link
Contributor

Size 1 reductions do not intiialize the iterator fully as normal
reductions which triggers uninitialized use in NpyIter_IsFirstVisit.
To fix this check the size of the iterator and return true if it only
has size 1.

Size 1 reductions do not intiialize the iterator fully as normal
reductions which triggers uninitialized use in NpyIter_IsFirstVisit.
To fix this check the size of the iterator and return true if it only
has size 1.
@charris
Copy link
Member

charris commented Mar 23, 2014

LGTM, thanks Julian.

charris added a commit that referenced this pull request Mar 23, 2014
BUG: fix uninitialized use of size 1 reductions
@charris charris merged commit e5b803c into numpy:master Mar 23, 2014
@juliantaylor juliantaylor deleted the itersize-1 branch March 23, 2014 16:36
@juliantaylor
Copy link
Contributor Author

I wonder if we should backport it, due to the iterator changes the valgrind warnings have disappeared so maybe its not even required anymore

seberg added a commit to seberg/numpy that referenced this pull request Mar 23, 2014
The FirstVisit function uses this, but when the reduction is
over a single element it isn't considered a reduction. This is
fine, however the reduce pos must still be initialized to 0 in
that case. Also changes the check order so that it is not
necessary to initialize the outerstrides as well.

See also numpygh-4134, and numpygh-4535.
@charris
Copy link
Member

charris commented Mar 23, 2014

I'm getting a bit lost on this subject. AFAIK, there have been two changes not backported, this and Sebastian's PR which reverts part of this. Both are pretty small, so I'd be inclined to backport both and maybe squash the two commits. And when that's done, I may have got vagrant figured out and running ;)

@certik BTW, I'm thinking it might be worth making numpy-vendor a numpy repository so we can keep it updated.

juliantaylor pushed a commit to juliantaylor/numpy that referenced this pull request Mar 23, 2014
The FirstVisit function uses this, but when the reduction is
over a single element it isn't considered a reduction. This is
fine, however the reduce pos must still be initialized to 0 in
that case. Also changes the check order so that it is not
necessary to initialize the outerstrides as well.

See also numpygh-4134, and numpygh-4535.
@juliantaylor
Copy link
Contributor Author

mine "fix" was obsoleted by sebers, I'll backport it
when do you wan to make the release?
I can create the binaries tomorrow, though if you are using my fork of the vendor you should get the same result

@rgommers
Copy link
Member

@charris https://github.com/certik/numpy-vendor contains binaries originally taken from https://github.com/numpy/vendor, so merging those two repos is the way to go.

@charris
Copy link
Member

charris commented Mar 23, 2014

@juliantaylor Tomorrow would be fine. I'm just starting to get vagrant figured out, so if you would make the binaries that would help.

@rgommers I looks like numpy-vendor wasn't a fork of vendor, so merging will require a bit of work. There are instructions at http://blog.caplin.com/2013/09/18/merging-two-git-repositories/comment-page-1/ .

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

3 participants