Ticket #6606 (closed: fixed)

Opened 8 years ago

Last modified 5 years ago

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:1 Changed 8 years ago by Russell Taylor

  • Status changed from new to accepted

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

comment:8 Changed 8 years ago by Owen Arnold

  • Status changed from verifying to closed

Mac tests passed after null pointer checked was fixed.

comment:9 Changed 5 years ago by Stuart Campbell

This ticket has been transferred to github issue 7452

Note: See TracTickets for help on using tickets.