Fix buffer overflow in TransformationListListNC

[ChrisJefferson]

Fix a buffer overflow in TransformationListList. This bug is in 4.10 (might be in earlier releases too)

Text for release notes

• Fix bug in TransformationListList which could cause a crash.

[ChrisJefferson]

@james-d-mitchell : I realise this is a busy time of year, just wanted to link you to this in case you had any opinion about it / I'd done anything stupid.

[coveralls]

Coverage remained the same at 85.169% when pulling da7909c on ChrisJefferson:fix-trans into 8e281e4 on gap-system:master.

[codecov]

Codecov Report

❗ No coverage uploaded for pull request base (master@8e281e4). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master    #3463   +/-   ##
=========================================
  Coverage          ?   85.34%           
=========================================
  Files             ?      699           
  Lines             ?   346475           
  Branches          ?        0           
=========================================
  Hits              ?   295685           
  Misses            ?    50790           
  Partials          ?        0

Impacted Files	Coverage Δ
src/trans.cc	99.8% <100%> (ø)

[fingolfin]

Two minor code formatting changes, please.
More importantly, I don't understand why this fixes a buffer overflow. It seems that this change just avoids an unnecessary assignment. So clearly I am missing something. Would you mind giving me a hint? Unless it is super trivial in the end, perhaps there should be a code comment?

[ChrisJefferson]

woops, done some clang-formatting.

The problem (I've tweaked the description to hopefully help) is that at the beginning of the function when we figure out the size of the transformation we need, we basically take the max of the values in the two arrays, EXCEPT we skip any index i where the two lists are equal. This makes sense, because if the transformation maps x to x for any x, we don't need to explicitly store that fact, as that's the default for transformations.

Except, if (say) the original arrays were [[1,1000], [2,1000]], we would decide we need a transformation of size 2, but then later try to fill in that the transformation maps 1000 to 1000, just writing into the memory past the end of the transformation.

This almost always won't be into another allocated bag (because we've only just allocated this memory), but it will make some future allocated bag be allocated with non-zero values, which is (in practice) basically as bad.

[ChrisJefferson]

also chucked a comment in

[james-d-mitchell]

This makes sense to me @ChrisJefferson, well spotted!

[fingolfin]

Thank you, that explanation makes perfect sense.