Skip to content

detect: fix SCTime_t -Wshorten-64-to-32 warnings#13189

Closed
catenacyber wants to merge 1 commit intoOISF:masterfrom
catenacyber:warnint-64to32-6186-v25.1
Closed

detect: fix SCTime_t -Wshorten-64-to-32 warnings#13189
catenacyber wants to merge 1 commit intoOISF:masterfrom
catenacyber:warnint-64to32-6186-v25.1

Conversation

@catenacyber
Copy link
Contributor

Link to redmine ticket:
https://redmine.openinfosecfoundation.org/issues/6186

Describe changes:

  • fix SCTime_t -Wshorten-64-to-32 warnings for remaining files : detect

@jlucovsky how does that look to you ? (as you did 31793af )

#13159 next iteration with new warnings getting fixed

Still to do afterwards :

  • fix other detect warnings
  • CI check

Ticket: OISF#6186

SCTime_t has seconds defined as 44 bits of a u64, so we use u64
for everything in seconds now.
@catenacyber catenacyber requested a review from victorjulien as a code owner May 8, 2025 20:07
@catenacyber catenacyber marked this pull request as draft May 8, 2025 20:42
@catenacyber
Copy link
Contributor Author

Draft : some format strings to fix also apparently

@codecov
Copy link

codecov bot commented May 8, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.12%. Comparing base (97eaeef) to head (b57d322).

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #13189      +/-   ##
==========================================
+ Coverage   83.11%   83.12%   +0.01%     
==========================================
  Files         988      988              
  Lines      272222   272222              
==========================================
+ Hits       226253   226290      +37     
+ Misses      45969    45932      -37     
Flag Coverage Δ
fuzzcorpus 61.41% <0.00%> (+0.03%) ⬆️
livemode 18.95% <100.00%> (+0.02%) ⬆️
pcap 44.87% <0.00%> (+<0.01%) ⬆️
suricata-verify 64.90% <0.00%> (-0.01%) ⬇️
unittests 58.14% <0.00%> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@victorjulien
Copy link
Member

Many of the structs use a uint32_t for secs to save space. So using a uint64_t will grow the structs w/o a real benefit.

If u32 isn't enough, we need to convert to SCTime_t instead of uint64_t

@catenacyber
Copy link
Contributor Author

Many of the structs use a uint32_t for secs to save space. So using a uint64_t will grow the structs w/o a real benefit.

If u32 isn't enough, we need to convert to SCTime_t instead of uint64_t

So, the question is wether they should keep u32 to save space but then check overflows, or use SCTime_t/uint64_t

The benefit here is to have a correct computation when u32 overflows.

SCTime_t is also a uint64_t with better sub-second granularity

I try next branch with SCTime_t

@catenacyber
Copy link
Contributor Author

Next in #13203

@catenacyber catenacyber closed this May 9, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants