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-icmp-seq: convert unittests to FAIL/PASS APIs #8080
detect-icmp-seq: convert unittests to FAIL/PASS APIs #8080
Conversation
I see the |
If there isn't one yet, yes, that's a good idea :) |
return 1; | ||
} | ||
return 0; | ||
FAIL_IF_NOT(iseq != NULL && htons(iseq->seq) == 300); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
please break this into 2 checks
return 1; | ||
} | ||
return 0; | ||
FAIL_IF_NOT(iseq != NULL && htons(iseq->seq) == 300); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
same
} | ||
return 1; | ||
FAIL_IF_NOT_NULL(iseq); | ||
DetectIcmpSeqFree(NULL, iseq); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
useless statement? It will only be called with iseq == NULL
@@ -366,11 +362,9 @@ static int DetectIcmpSeqParseTest03 (void) | |||
{ | |||
DetectIcmpSeqData *iseq = NULL; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
in your next PR, could you change these to a single line, e.g.
DetectIcmpSeqData *iseq = DetectIcmpSeqParse(NULL, "badc");
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
see inline comments
Thanks for the feedback, I will make the changes |
Task: #4043
Make sure these boxes are signed before submitting your Pull Request -- thank you.
Link to redmine ticket: https://redmine.openinfosecfoundation.org/issues/4043
Describe changes: