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

detect-rpc-v1 #5590

Closed
wants to merge 1 commit into from
Closed

detect-rpc-v1 #5590

wants to merge 1 commit into from

Conversation

jufajardini
Copy link
Contributor

  • detect-rpc: convert unit tests to new FAIL/PASS API.
  • fix unused variable error.

Make sure these boxes are signed before submitting your Pull Request -- thank you.

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

Previous PR:
#5589

Describe changes:

  • detect-rpc: convert unit tests to new FAIL/PASS API.
  • detect-rpc: fix unused variable error.

PRScript output (if applicable):

#suricata-verify-pr:
#suricata-verify-repo:
#suricata-verify-branch:
#suricata-update-pr:
#suricata-update-repo:
#suricata-update-branch:
#libhtp-pr:
#libhtp-repo:
#libhtp-branch:

- detect-rpc: convert unit tests to new FAIL/PASS API.
- fix unused variable error.
@jufajardini jufajardini requested a review from a team as a code owner November 22, 2020 13:52
@jufajardini
Copy link
Contributor Author

Copying from previous, closed PR: I was wondering if some of the longer tests (like 3rd and 5th) shouldn't be split into smaller, more concise ones...
Also I am not sure about the debugging messages. I mean, they wouldn't work as they did in the previous code, because there are no checks any more, that's why I deleted them. But... should they go somewhere else, now? That's another reason for me to think that splitting the tests could be a good thing. Would be easier to know what was the matter, when a test failed...

@jufajardini jufajardini mentioned this pull request Nov 22, 2020
3 tasks
if (de_ctx == NULL) {
goto end;
}
FAIL_IF_NULL(de_ctx);

de_ctx->flags |= DE_QUIET;

s = de_ctx->sig_list = SigInit(de_ctx,"alert udp any any -> any any (msg:\"RPC Get Port Call\"; rpc:100000, 2, 3; sid:1;)");
Copy link
Member

Choose a reason for hiding this comment

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

Can you make an additional cleanup here?
s = de_ctx->sig_list = SigInit( can be turned into s = DetectEngineAppendSig(

Copy link
Member

Choose a reason for hiding this comment

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

(same for the others)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, done!

@jufajardini jufajardini mentioned this pull request Nov 22, 2020
3 tasks
@jufajardini
Copy link
Contributor Author

Closed with #5591

@jufajardini jufajardini deleted the detect-rpc-v2 branch July 1, 2021 16:26
hsadia538 added a commit to hsadia538/suricata that referenced this pull request Oct 24, 2022
victorjulien pushed a commit to victorjulien/suricata that referenced this pull request Oct 27, 2022
benignbala pushed a commit to benignbala/suricata that referenced this pull request Nov 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants