Conversation
instead of a global variable. For easier initialization with dynamic number of protocols
for expectation_proto Ticket: 5053
for alproto_names Ticket: 5053
Ticket: 5053
Ticket: 5053 delay after initialization so that StringToAppProto works
so that we can use safely EXCEPTION_POLICY_MAX*sizeof(x)
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #11701 +/- ##
==========================================
- Coverage 82.63% 82.43% -0.21%
==========================================
Files 919 919
Lines 248925 248932 +7
==========================================
- Hits 205703 205208 -495
- Misses 43222 43724 +502
Flags with carried forward coverage won't be shown. Click here to find out more. |
|
ERROR: ERROR: QA failed on SURI_TLPR1_alerts_cmp. Pipeline 22322 |
jufajardini
left a comment
jufajardini
left a comment
There was a problem hiding this comment.
Didn't manage to review everything, leaving minor comments.
(the inversion protomap<->alproto for app-layer is mind fogging 😅 )
| FatalError("Unable to alloc alproto_names."); | ||
| } | ||
| // to realloc when dynamic protos are added | ||
| alpd_ctx.expectation_proto = SCCalloc(ALPROTO_MAX, sizeof(uint8_t)); | ||
| if (unlikely(alpd_ctx.expectation_proto == NULL)) { | ||
| FatalError("Unable to alloc expectation_proto."); |
There was a problem hiding this comment.
Should these errors have some more user-friendly part?
There was a problem hiding this comment.
How so ?
This is a unique message showing clearly the problem, is it not ?
There was a problem hiding this comment.
Hm, I guess, maybe. One could always copy the error and share that with us, as many users probably wouldn't try to fix something like that...
| DEBUG_VALIDATE_BUG_ON((int64_t)(SCTIME_SECS(f->lastts) - SCTIME_SECS(f->startts)) < INT32_MIN || | ||
| (int64_t)(SCTIME_SECS(f->lastts) - SCTIME_SECS(f->startts)) > INT32_MAX); | ||
| int32_t age = (int32_t)(SCTIME_SECS(f->lastts) - SCTIME_SECS(f->startts)); |
There was a problem hiding this comment.
Since this is repeated a few times, couldn't it be wrapped in a function?
There was a problem hiding this comment.
Oops, this commit should not be in this PR
| }; | ||
| /* max used in app-layer (counters) */ | ||
| #define FLOW_PROTO_APPLAYER_MAX FLOW_PROTO_UDP + 1 | ||
| #define FLOW_PROTO_APPLAYER_MAX (FLOW_PROTO_UDP + 1) |
There was a problem hiding this comment.
Both are possible.
As a matter of facts, I did c32bc61 after finding this parenthesis addition was needed for 897b6606e4ae116337ce9899d34c7ab9c6a14fda
Do you have a better C way to do this ? |
|
Next in #11795 |
Nothing that comes easily. Was just a silly comment. |
Not silly, I felt the pain, but found no other way... |
3 participants
Link to ticket: https://redmine.openinfosecfoundation.org/issues/
Preliminary work for https://redmine.openinfosecfoundation.org/issues/5053
Describe changes:
#11572 next round
#11696 with clean git history and green CI
Still more work to do after :
Only
AppProtoStringsis to be handled, but it is the big one.And then take remaining commits out of #11321
And supply an example of an app-layer plugin