Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Applayer plugin 5053 v3.15.1 #12234

Closed

Conversation

catenacyber
Copy link
Contributor

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

Describe changes:

  • get ready to use dynamic number of app-layer protos (also work with static constant) in all places
  • preventive fix of macro with parenthesis cc @jufajardini

Let me know if I should split this PR

#12163 first commits, just making alprotos number dynamic, and not yet adding plugins support

instead of a global variable.

For easier initialization with dynamic number of protocols
for expectation_proto

Ticket: 5053
so that we can use safely EXCEPTION_POLICY_MAX*sizeof(x)
Ticket: 5053

delay after initialization so that StringToAppProto works
Copy link

codecov bot commented Dec 5, 2024

Codecov Report

Attention: Patch coverage is 91.14583% with 17 lines in your changes missing coverage. Please review.

Project coverage is 83.07%. Comparing base (09ba69c) to head (c53a564).
Report is 46 commits behind head on master.

Additional details and impacted files
@@            Coverage Diff             @@
##           master   #12234      +/-   ##
==========================================
- Coverage   83.19%   83.07%   -0.12%     
==========================================
  Files         912      912              
  Lines      257166   268555   +11389     
==========================================
+ Hits       213938   223101    +9163     
- Misses      43228    45454    +2226     
Flag Coverage Δ
fuzzcorpus 62.01% <81.15%> (+1.00%) ⬆️
livemode 19.51% <71.20%> (+0.10%) ⬆️
pcap 46.34% <84.81%> (+1.94%) ⬆️
suricata-verify 64.47% <91.09%> (+1.69%) ⬆️
unittests 58.28% <70.83%> (-0.91%) ⬇️

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

@suricata-qa
Copy link

Information:

ERROR: QA failed on SURI_TLPR1_alerts_cmp.

field baseline test %
SURI_TLPR1_stats_chk
.app_layer.flow.dcerpc_tcp 40 42 105.0%

Pipeline 23707

@victorjulien
Copy link
Member

replaced by #12256

@catenacyber
Copy link
Contributor Author

Did you mean you do not want the PR split ?
If so, do you also want #12233 closed as taken in #12256 ?

@suricata-qa
Copy link

Information: QA ran without warnings.

Pipeline 24007

@victorjulien
Copy link
Member

Confused, why is there a v15 open as well as a v16?

@catenacyber
Copy link
Contributor Author

Confused, why is there a v15 open as well as a v16?

TL;DR
This PR and #12307 are the current ones

v15.1 was a part of v16

I could have made a v16.1 which is v15.1 rebased, but not wasting resources

As QA showed something interesting cf #12307 I am closing #12256

Let me know if

@catenacyber
Copy link
Contributor Author

Rebased in #12352

@catenacyber catenacyber closed this Jan 7, 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.

3 participants