Ticket #6606 (closed: fixed)
Remove unsafe V3D constructor by pointer
| Reported by: | Russell Taylor | Owned by: | Russell Taylor |
|---|---|---|---|
| Priority: | critical | Milestone: | Release 2.5 |
| Component: | Mantid | Keywords: | |
| Cc: | Blocked By: | ||
| Blocking: | Tester: | Owen Arnold |
Description
We now have a failing test: MDEventsTest_Integrate3DEventsTest
Change History
comment:2 Changed 8 years ago by Russell Taylor
- Summary changed from The upgrade of the boost version has broken the mac build to Remove unsafe V3D constructor by pointer
OK, this turns out not to be directly caused by the boost upgrade, but just shown up by this - so I've changed the ticket title.
The problem is that the V3D(double*) constructor is selected when code like if ( my_V3D != 0 ) is written. This is probably not what is intended. It would work if the constructor correctly initialized the elements to zero if the pointer is null (which it does check for), but it doesn't. That would be easy to fix, but even then the constructor is unsafe because you could pass in an array of the wrong length and there'd be no way to detect that. So I've made the decision to remove the constructor entirely (it's only used in a few places).
comment:3 Changed 8 years ago by Russell Taylor
Re #6606. Indentation fix only - no code changes.
Changeset: 6eb5e8b3dde9fa9e2c26e4a2517aff5c81621a56
comment:4 Changed 8 years ago by Russell Taylor
Re #6606. Indentation fix only - no code changes.
Changeset: b1650df1f1fc49097cecceb130f9c477724343cb
comment:5 Changed 8 years ago by Russell Taylor
Re #6606. Remove unsafe V3D constructor from a pointer.
Although it checked for a null pointer being passed in, it didn't initialise the vector members in that case. Even if that were fixed is would be unsafe as an array of the wrong length could be passed in. It was only used in a few places, so I've elected to just remove it.
This should fix the failing test on the ORNL mac build.
Changeset: 698417df74593c7aad917d04d3e7b512f99f92be
comment:6 Changed 8 years ago by Russell Taylor
- Status changed from accepted to verify
- Resolution set to fixed
Testing:
This problem was causing the MDEventsTest_Integrate3DEventsTest to fail consistently on the Mac, so observe that this is no longer the case.
The specific line that had a bug was line 396 of Integrate3DEvents.cpp: if ( peak_q != 0 )
The right hand side of this needs to create a V3D and did so by going to the V3D(double*) constructor and passing in a null pointer. Although the constructor did check for a null pointer, it then failed to initialise the members of the vector. I changed the line to use the existing V3D::nullVector() method.
comment:7 Changed 8 years ago by Owen Arnold
- Status changed from verify to verifying
- Tester set to Owen Arnold
