Applayer plugin 5053 v3.5

[catenacyber]

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

Describe changes:

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

#11572 next round

#11795 with comments taken into account

Still more work to do after :
Only AppProtoStrings is 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

[codecov]

Codecov Report

Attention: Patch coverage is 90.52632% with 18 lines in your changes missing coverage. Please review.

Project coverage is 82.42%. Comparing base (6ae5ae7) to head (b03265b).
Report is 41 commits behind head on master.

Additional details and impacted files

@@            Coverage Diff             @@
##           master   #11910      +/-   ##
==========================================
- Coverage   82.60%   82.42%   -0.19%     
==========================================
  Files         912      912              
  Lines      249342   249350       +8     
==========================================
- Hits       205968   205519     -449     
- Misses      43374    43831     +457     

Flag	Coverage Δ
fuzzcorpus	59.84% <80.85%> (-0.79%)	⬇️
livemode	18.87% <69.14%> (+0.15%)	⬆️
pcap	44.08% <84.04%> (+<0.01%)	⬆️
suricata-verify	61.99% <90.42%> (-0.04%)	⬇️
unittests	58.93% <69.47%> (+<0.01%)	⬆️

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

[suricata-qa]

Information: QA ran without warnings.

Pipeline 23058

[catenacyber]

There are still discussions:

• Should we use void *(*alproto_local_storage)[FLOW_PROTO_MAX]; alproto_local_storage[alproto][flow_proto] or void **alproto_local_storage; alproto_local_storage[alproto*FLOW_PROTO_MAX+flow_proto] Applayer plugin 5053 v3.4 #11795 (comment)
• Could we remove ALPROTO_TEST as the tests do not make sense to me ?

[catenacyber]

Replaced by #11987

[victorjulien]

Could we remove ALPROTO_TEST as the tests do not make sense to me ?

What about them doesn't make sense?

[catenacyber]

Could we remove ALPROTO_TEST as the tests do not make sense to me ?

What about them doesn't make sense?

Registering a alproto with a parser function that always fails, and just testing that AppLayerParserParse returned -1. Would you not get the same result without registering a parser function, or using ALPROTO_FAILED as argument to AppLayerParserParse ?

The comment says "Test the deallocation of app layer parser memory on occurrence of error in the parsing process." but I do not see such a thing, is there ?

No big deal, but I do not see the point of these tests...

[victorjulien]

Could we remove ALPROTO_TEST as the tests do not make sense to me ?

What about them doesn't make sense?

Registering a alproto with a parser function that always fails, and just testing that AppLayerParserParse returned -1. Would you not get the same result without registering a parser function, or using ALPROTO_FAILED as argument to AppLayerParserParse ?

The comment says "Test the deallocation of app layer parser memory on occurrence of error in the parsing process." but I do not see such a thing, is there ?

No big deal, but I do not see the point of these tests...

I'm fine with removing them.

[catenacyber]

I'm fine with removing them.

Done in #12051