Bug Fix: The error checking in PartialPerm has been corrected such that invalid inputs (numbers < 1) are detected #3944
Conversation
ChrisJefferson
added
release notes: to be added
| if IsSSortedList(arg[1]) and ForAll(arg[1], IsSmallIntRep) | ||
| and IsDuplicateFreeList(arg[2]) and ForAll(arg[2], IsSmallIntRep) | ||
| if IsSSortedList(arg[1]) and ForAll(arg[1], IsPosInt and IsSmallIntRep) | ||
| and IsDuplicateFreeList(arg[2]) and ForAll(arg[2], IsPosInt and IsSmallIntRep) | ||
| and Length(arg[1]) = Length(arg[2]) then | ||
| return SparsePartialPermNC(arg[1], arg[2]); |
There was a problem hiding this comment.
Strictly speaking I'd feel better if we improved the input validation in SparsePartialPermNC instead or in addition to the above, but as this PR fixes the immediate issue, I'd be happy to see it merged as-is, too.
There was a problem hiding this comment.
The small pos int and "sorted list" tests can be done very cheaply in the kernel; duplicate free might be more work, though perhaps even that could be done as a side effect of the kernel code. Anyway, as long as there is no performance issue here... shrug
There was a problem hiding this comment.
I agree porting this all into the kernel might be netter, most of this comes almost for free while you are building the partial permutation (except DuplicateFreeList), but that would be a bigger change, and this little fix feels better for backporting.
|
Backported to stable-4.11 in aac1175 |
danielrademacher
changed the title
danielrademacher
added
release notes: added
ThomasBreuer
added
the
kind: bug
Fixes #3943 , corruption caused by invalid inputs (numbers < 1) to PartialPerm