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

src: rework includes as per cppclean #7563

Closed
wants to merge 2 commits into from

Conversation

catenacyber
Copy link
Contributor

Link to redmine ticket:
Is there already one @jasonish ?

Describe changes:

  • Rework includes as per cppclean

Still to do : add cppclean as part of CI

I used this patched version myint/cppclean#165

There are still exceptions :

  • About Suricata : When "rust.h" or "suricata-common.h" gets included, and when includes is rust.h or rust-context.h
  • About cppclean
    • when using macros such as TAILQ with including queue.h (or red and black tree)
    • when using the array size, such as ``uint8_t plugin_v[PLUGIN_VAR_SIZE]; in decode.h needing suricata-plugin.h

The result is less includes from header file to another, nothing is done for C files including a useless header

@suricata-qa
Copy link

WARNING:

field test baseline %
build_asan

Pipeline 7955

@jasonish
Copy link
Member

Is there already one @jasonish ?

No, any include cleanups I've done in the past have been related to other fixups, so no specific include cleanup ticket yet.

@jasonish
Copy link
Member

Would be great if we could get this to work, as in remove all unecessary includes from headers, then source files. Would make it much easier to proceed with some manual cleanup, and logical organization with respect to libsuricata and what we consider private and public.

@suricata-qa
Copy link

WARNING:

field test baseline %
build_asan

Pipeline 7962

@codecov
Copy link

codecov bot commented Jun 21, 2022

Codecov Report

Merging #7563 (782e67a) into master (a898409) will decrease coverage by 0.05%.
The diff coverage is n/a.

❗ Current head 782e67a differs from pull request most recent head 5c78ffc. Consider uploading reports for the commit 5c78ffc to get more accurate results

@@            Coverage Diff             @@
##           master    #7563      +/-   ##
==========================================
- Coverage   75.80%   75.75%   -0.06%     
==========================================
  Files         658      658              
  Lines      186526   186522       -4     
==========================================
- Hits       141399   141297     -102     
- Misses      45127    45225      +98     
Flag Coverage Δ
fuzzcorpus 59.88% <ø> (-0.08%) ⬇️
suricata-verify 52.36% <ø> (-0.06%) ⬇️
unittests 60.72% <ø> (-0.01%) ⬇️

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

@suricata-qa
Copy link

WARNING:

field test baseline %
build_asan

Pipeline 7965

@suricata-qa
Copy link

WARNING:

field test baseline %
build_asan

Pipeline 7966

@suricata-qa
Copy link

WARNING:

field test baseline %
build_asan

Pipeline 7967

@suricata-qa
Copy link

WARNING:

field test baseline %
build_asan

Pipeline 7968

@suricata-qa
Copy link

WARNING:

field test baseline %
build_asan

Pipeline 7970

@suricata-qa
Copy link

WARNING:

field test baseline %
build_asan

Pipeline 7973

@suricata-qa
Copy link

WARNING:

field test baseline %
build_asan

Pipeline 7974

@suricata-qa
Copy link

ERROR:

ERROR: QA failed on ips_afp_drop_chk.

Pipeline 7975

@suricata-qa
Copy link

ERROR:

ERROR: QA failed on ips_afp_drop_chk.

Pipeline 7979

@@ -24,11 +24,6 @@
#ifndef __APP_LAYER_NBSS_H__
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

looks like this file can be removed instead? Should have been removed as part of the smb to rust work years ago I think

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh and so are decode-null.h and util-vector.h :-)

@victorjulien
Copy link
Member

Looks good to me. Will run in my own ci setup as well to get a few more build types

@victorjulien
Copy link
Member

  CC       util-mpm-hs.o
util-mpm-hs.c: In function 'SCHSTest29':
util-mpm-hs.c:2118:5: error: implicit declaration of function 'SigGroupBuild' [-Werror=implicit-function-declaration]
 2118 |     SigGroupBuild(de_ctx);
      |     ^~~~~~~~~~~~~
util-mpm-hs.c:2134:9: error: implicit declaration of function 'SigGroupCleanup' [-Werror=implicit-function-declaration]
 2134 |         SigGroupCleanup(de_ctx);
      |         ^~~~~~~~~~~~~~~
util-mpm-hs.c:2135:9: error: implicit declaration of function 'SigCleanSignatures'; did you mean 'SigLoadSignatures'? [-Werror=implicit-function-declaration]
 2135 |         SigCleanSignatures(de_ctx);
      |         ^~~~~~~~~~~~~~~~~~
      |         SigLoadSignatures

@catenacyber catenacyber mentioned this pull request Jun 27, 2022
@catenacyber
Copy link
Contributor Author

Replaced by #7570 fixing util-mpm-hs.c to include detect-engine-build.h (and also in detect-dce-iface.c detect-dce-opnum.c tests/detect.c)

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.

4 participants