Project

General

Profile

Actions

Development #1388

closed

Development #1353: Code cleanup for OCT19

Code cleanup: fles/reader/unpacker/CbmTSUnpackStsxyter.cxx

Added by Volker Friese almost 3 years ago. Updated almost 3 years ago.

Status:
Closed
Priority:
Normal
Assignee:
Target version:
Start date:
09/21/2019
Due date:
09/27/2019
% Done:

100%

Estimated time:
1.00 h
Spent time:

Description

Compiler warning (gcc):

/home/uhlig/cbm/release/oct19/fles/reader/unpacker/CbmTSUnpackStsxyter.cxx: In member function 'void CbmTSUnpackStsxyter::UnpackHitMessage(const uint32_t&, const Int_t&, const uint16_t&)':
[CTest: warning matched] /home/uhlig/cbm/release/oct19/fles/reader/unpacker/CbmTSUnpackStsxyter.cxx:400:113: warning: comparison between signed and unsigned integer expressions [-Wsign-compare]
     Bool_t parityError = ( (GetEvenParity( time ) ^ GetEvenParity( amplitude ) ^ GetEvenParity( hitMissed) ^ 1) == parity );

Are you sure that the code does what you want to? Exclusive-or with three operands... The result will compute to 1 if one of the operands is true and the other two are false, but also if all three are true.

Anyhow, the bitwise operator ^seems to convert your bools into integer, which is then compared to the unsigned integer parity. Please check and fix.

Actions #1

Updated by Pierre-Alain Loizeau almost 3 years ago

Huuum, not sure, I just following the recommendation of the g++ warning about the danger of exclusive or combined with an equality OP without parenthesis.

From what I can see this is one of these class which were created years ago and not touched by since then. Maybe Florian knows what was attempted here.

Actions #2

Updated by Volker Friese almost 3 years ago

  • Status changed from Assigned to In Progress
  • Assignee changed from Pierre-Alain Loizeau to Florian Uhlig
Actions #3

Updated by Florian Uhlig almost 3 years ago

  • Assignee changed from Florian Uhlig to Volker Friese
  • % Done changed from 0 to 50

I don't know what should be done here. Since the calculated bool is only needed in the constructor of the data class CbmStsXyterRawData and this data class is not used anywhere in CbmRoot I decided to hard code the parity value to get rid of the warning.

The proper way to handle this is to find out if someone really needs the data class and the unpacker and if not remove both classes.

Actions #4

Updated by Volker Friese almost 3 years ago

  • % Done changed from 50 to 80

Your fix cured the warning in my local build. Let's wait for CDash.

There will be a general revision of the beamtime classes (directories beamtime and fles); surely, we have to remove obsolete software here.

Actions #5

Updated by Volker Friese almost 3 years ago

  • Status changed from In Progress to Closed
  • % Done changed from 80 to 100

Fixed.

Actions

Also available in: Atom PDF