BUG: Fix small valgrind-found issues #18651
Conversation
This should be backportable. There was at least one that I could not reproduce when running the tests again. And the new random-shuffle tests give false-positives (which is just slightly annoying, considering that we are very close to almost only "longdouble" related false-positives)
The missing decref here only leaks references and can never leak actual memory fortunately.
| if (dict == NULL) { | ||
| return NULL; | ||
| } | ||
| /**begin repeat | ||
| * #str = func, var, func_xb, var_xb# | ||
| */ | ||
| item = PyUnicode_FromString(highest_@str@); |
There was a problem hiding this comment.
This looks like the original was OK. Could move the declaration/assignment down here.
There was a problem hiding this comment.
The code is in a /**begin repeatloop. I missed that also first, but that means it gets repeated 3 times. The (completely fine) alternative would be to put a Py_DECREF at the end of the loop.
Which, in hindsight is probably prettier...
There was a problem hiding this comment.
Pushed a commit to simplifiy this, no need for the big SETREF gun...
| @@ -7486,7 +7486,7 @@ def test_out_of_order_fields(self): | |||
| memoryview(arr) | |||
|
|
|||
| def test_max_dims(self): | |||
| a = np.empty((1,) * 32) | |||
There was a problem hiding this comment.
Was there a problem with empty? I agree ones is nicer.
There was a problem hiding this comment.
Its a harmless "use of uninitialized memory" in valgrind because check_roundtrip actually does something with the values (or at least it looks like it).
Using SETREF can be nice, but was just overcomplicating thing here...
|
I cannot figure out what this does with the item (list), |
|
Thanks Sebastian. |
|
Oh, yeah. valgrind seemed happy, so I was. I missed that |
* BUG: Fix small valgrind-found issues This should be backportable. There was at least one that I could not reproduce when running the tests again. And the new random-shuffle tests give false-positives (which is just slightly annoying, considering that we are very close to almost only "longdouble" related false-positives) * BUG: Add missing decref in user-dtype fallback paths The missing decref here only leaks references and can never leak actual memory fortunately. * MAINT,TST: Simplify the "refcount logic" in the dispatch tests again Using SETREF can be nice, but was just overcomplicating thing here...
BUG: Fix small valgrind-found issues (#18651)
This should be backportable. There was at least one that I could
not reproduce when running the tests again. And the new random-shuffle
tests give false-positives (which is just slightly annoying, considering
that we are very close to almost only "longdouble" related
false-positives)
I don't think this touches any code modified in the main branch. So it should be backported to 1.20. I may have to run pytest-leaks... (although leaks found there but not with valgrind are unlikely to cause much trouble in the wild...)