FIX: protect stolen ref by PyArray_NewFromDescr in array_empty #8180
Conversation
|
LGTM. The description of |
|
I don't think a test is needed, but @charris may want the commit message to say "BUG" instead of "FIX", see our standard prefixes, so I'll leave that to him. Edit: And the commit message should say "fixes #8179" |
| @@ -2895,20 +2895,25 @@ PyArray_Empty(int nd, npy_intp *dims, PyArray_Descr *type, int is_f_order) | |||
| PyArrayObject *ret; | |||
|
|
|||
| if (!type) type = PyArray_DescrFromType(NPY_DEFAULT_TYPE); | |||
|
|
|||
| /* PyArray_NewFromDescr steals a ref, | |||
| * but we need to look at type later. */ | |||
There was a problem hiding this comment.
While we are at nitpicking, the comment end should be formatted:
/*
* Text here
* more text
*/
|
Should try your valgrind commands, or is there some speical setup needed? For some reason, running numpy/python stuff through valgrind does not work for me anymore since a while.... |
|
Out of curiosity, did you run with a specially compiled python or do those valgrind options hide the typical python allocation spam? |
|
@seberg For one, I updated to Fedora 24 and all of the allocation spam are gone. gdb supports python too. If that counts towards an answer... |
|
@ahaldane messages updated. |
|
@seberg : according to the changelog at http://koji.fedoraproject.org/koji/buildinfo?buildID=809980, Fedora has been compiling Python with valgrind support for a few years. The support may have just gotten better in 3.5, I guess. |
|
Dunno, was using debian/ubuntu, it always worked (though I had to add suppressions, which probably would have hidden cases such as this one as well), have not tried it in a bit, but more recently things always crashed. But no worries, seems like other people run the testsuit through valgrind once in a while nowadays ;). I'll leave the merge to @ahaldane. |
|
Great, merging. Thanks @rainwoodman for the valgrind tests! |
|
no problems! |
This fixes #8179.
Valgrind warning is gone.
I checked other uses of PyArray_NewFromDescr in ctor.c all look healthy.