Cppcheck/v1 #11946
Cppcheck/v1 #11946
Conversation
This is a more compact time format.
It tripped up cppcheck:
src/util-time.h:124:35: error: syntax error [syntaxError]
return !timercmp(first, second, >);
Clang 19:
util-hash-string.c:41:16: error: implicit conversion loses integer precision: 'unsigned long' to 'int' [-Werror,-Wshorten-64-to-32]
41 | int len1 = strlen((char *)data1);
| ~~~~ ^~~~~~~~~~~~~~~~~~~~~
util-hash-string.c:42:16: error: implicit conversion loses integer precision: 'unsigned long' to 'int' [-Werror,-Wshorten-64-to-32]
42 | int len2 = strlen((char *)data2);
| ~~~~ ^~~~~~~~~~~~~~~~~~~~~
2 errors generated.
Issue warning if thread sleeps for a long time.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11946 +/- ##
==========================================
- Coverage 82.74% 82.71% -0.03%
==========================================
Files 912 912
Lines 249102 249110 +8
==========================================
- Hits 206117 206050 -67
- Misses 42985 43060 +75
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
Information: QA ran without warnings. Pipeline 23094 |
| SCMutexUnlock(&thread_store_lock); | ||
| } | ||
|
|
||
| bool TmThreadsTimeSubsysIsReady(void) | ||
| { | ||
| static SCTime_t nullts; |
There was a problem hiding this comment.
Should we use SCTIME_INITIALIZER just to be sure?
| if (t->sys_sec_stamp.secs != 0) { | ||
| SCTime_t tmpts = SCTIME_ADD_SECS(t->sys_sec_stamp, 3); | ||
| if (SCTIME_CMP_LT(tmpts, now)) { | ||
| SCLogNotice("%s: thread slept for %u secs", t->name, (uint32_t)(now.secs - tmpts.secs)); |
There was a problem hiding this comment.
Not sure actually, it's not necessarily an issue. In a large pcap that starts with a single flow you might see this once packets for other flows are processed. I'll turn it into debug for now.
| SCTime_t now = SCTIME_FROM_TIMEVAL(&tv); | ||
|
|
||
| if (t->sys_sec_stamp.secs != 0) { | ||
| SCTime_t tmpts = SCTIME_ADD_SECS(t->sys_sec_stamp, 3); |
There was a problem hiding this comment.
Is there a reason for using ~4 seconds == long time?
There was a problem hiding this comment.
it's ore a left over of some debugging where I wanted to know the effect of inactive threads on the timestamp the FM used. Turned it into debug.
| memset(&now, 0, sizeof(now)); | ||
| gettimeofday(&now, NULL); | ||
| struct timeval tv; | ||
| memset(&tv, 0, sizeof(tv)); |
There was a problem hiding this comment.
Is the memset needed? If yes, it seems to be missing in other places.
There was a problem hiding this comment.
We have it in other places, see
git grep -B1 gettimeofday|grep memset
src/source-ipfw.c- memset (&IPFWts, 0, sizeof(struct timeval));
src/source-nflog.c- memset(&tv, 0, sizeof(tv));
src/source-nfq.c- memset(&tv, 0, sizeof(tv));
src/suricata.c- memset(&suri->start_time, 0, sizeof(suri->start_time));
src/suricata.c- memset(&end_time, 0, sizeof(end_time));
src/util-random.c- memset(&tv, 0, sizeof(tv));
src/util-var-name.c- memset(&ts, 0, sizeof(ts));
src/util-var-name.c- memset(&tv, 0, sizeof(tv));
However I agree it's probably not needed as there are many more places were it's not done. Not sure how I developed this as a half baked pattern.
|
Replaced by #11967 |
Cppcheck 2.14 fixes plus a few misc improvements.