HLT GCC 7 Cleanup #17239
HLT GCC 7 Cleanup #17239
Conversation
TString in ROOT support std::string_view which was added into C++17 standard and CMSSW since GCC 6 is compiled with C++17. GCC 7 will provide a ful C++17 language support and most of C++ standard library. TString defines two conversion operators, one for const char * and one for std::string_view. The std::string can be constructor by both of them thus causing compiler to error on it. Use static_cast to explicitly select conversion operator which would be compatible with previous C++ standards. Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
According GCC the output string could be up to 315 chars long. Adjust buffer to accommodate that. Signed-off-by: David Abdurachmanov <David.Abdurachmanov@cern.ch>
|
A new Pull Request was created by @davidlt for CMSSW_9_0_X. It involves the following packages: HLTrigger/HLTanalyzers @perrotta, @cmsbuild, @silviodonato, @Martin-Grunewald, @fwyzard, @davidlange6 can you please review it and eventually sign? Thanks. cms-bot commands are listed here #13028 |
|
please test |
|
The tests are being triggered in jenkins. |
|
Comparison job queued. |
| @@ -1880,16 +1880,16 @@ int main(int argc, char ** argv) { | |||
| } | |||
| } | |||
|
|
|||
| char value[10] ; | |||
| char value[330] ; | |||
There was a problem hiding this comment.
According to the GCC the format can generate a string up to 316 bytes.
error: '%9.4f' directive output may be truncated writing between 9 and 315 bytes into a region of size 10 [-Werror=format-truncation=]
note: format output between 10 and 316 bytes into a destination of size 10
There was a problem hiding this comment.
I'm not sure I agree... anyway, why is this an error and not a warning ? snprintf should just truncate the output, right ?
There was a problem hiding this comment.
This is an error in GCC 7. It's a new flag. It's triggered for both, overflows and truncations. And yes, snprintf will just print just keep n-1 chars from the format string (1 char is needed for NULL termination). The buffer needs to be big, because we are printing double here. I checked at least DBL_MAX and that seems to be 308+ chars.
Looks like the warning/errors is not triggered if one checks for return value of snprintf. In that case GCC assumes that developer detected truncation and handled it correctly.
There was a problem hiding this comment.
The buffer needs to be big, because we are printing
doublehere
True, but we it looks like we are printing bin counts and/or execution times here... if the execution time needs 300 characters to print, it's unlikely I'll stick around to see it printed :-)
| @@ -557,7 +557,7 @@ void HLTInfo::analyze(const edm::Handle<edm::TriggerResults> & h | |||
| // ...Fill the corresponding accepts in branch-variables | |||
| l1flag[iBit] = gtDecisionWord[iBit]; | |||
|
|
|||
| std::string l1triggername= std::string (algoBitToName[iBit]); | |||
| std::string l1triggername= std::string (static_cast<const char *>(algoBitToName[iBit])); | |||
There was a problem hiding this comment.
sigh... I'm actually sorry for having this kind of code under HLTrigger/, even though it's not used in any production workflow (and possibly not used any more at all, since it doesn't look like it was ever ported to the L! trigger used in Run 2).
OK for the minimal fix, I'll add it to my list of things to be cleaned up (or dropped).
|
+1 |
cmsbuild
added
fully-signed
hlt-approved
and removed
hlt-pending
pending-signatures
labels
|
This pull request is fully signed and it will be integrated in one of the next CMSSW_9_0_X IBs (tests are also fine). This pull request requires discussion in the ORP meeting before it's merged. @davidlange6, @smuzaffar |
|
+1 |
Two commits cleaning up GCC 7 compilation errors in HLT packages.