-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Task 4053, convert unit tests to FAIL/PASS API #5497
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -125,89 +125,66 @@ static int DetectMsgSetup (DetectEngineCtx *de_ctx, Signature *s, const char *ms | |
#ifdef UNITTESTS | ||
static int DetectMsgParseTest01(void) | ||
{ | ||
int result = 0; | ||
Signature *sig = NULL; | ||
const char *teststringparsed = "flow stateless to_server"; | ||
DetectEngineCtx *de_ctx = DetectEngineCtxInit(); | ||
if (de_ctx == NULL) | ||
goto end; | ||
FAIL_IF(de_ctx == NULL); | ||
|
||
FILE *fd = SCClassConfGenerateValidDummyClassConfigFD01(); | ||
SCClassConfLoadClassficationConfigFile(de_ctx, fd); | ||
|
||
sig = SigInit(de_ctx, "alert tcp any any -> any any (msg:\"flow stateless to_server\"; flow:stateless,to_server; content:\"flowstatelesscheck\"; classtype:bad-unknown; sid: 40000002; rev: 1;)"); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. extra points for: declare There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Got it, thanks. Following this logic, could lines
Be moved to the end, where there used to be the check
I don't see fd being directly used anywhere else, which makes me believe they could. I have checked SCClassConfLoadClassficationConfigFile and it seems it will only use de_ctx, not change it. So it seems that this is something that could sort of be together with the other statements near
Does that make sense? |
||
if(sig == NULL) | ||
goto end; | ||
FAIL_IF(sig == NULL); | ||
|
||
if (strcmp(sig->msg, teststringparsed) != 0) { | ||
printf("got \"%s\", expected: \"%s\": ", sig->msg, teststringparsed); | ||
goto end; | ||
} | ||
FAIL_IF(strcmp(sig->msg, teststringparsed) != 0); | ||
|
||
result = 1; | ||
end: | ||
if (sig != NULL) | ||
jufajardini marked this conversation as resolved.
Show resolved
Hide resolved
|
||
SigFree(de_ctx, sig); | ||
if (de_ctx != NULL) | ||
DetectEngineCtxFree(de_ctx); | ||
return result; | ||
|
||
PASS; | ||
} | ||
|
||
static int DetectMsgParseTest02(void) | ||
{ | ||
int result = 0; | ||
Signature *sig = NULL; | ||
const char *teststringparsed = "msg escape tests wxy'\"\\;:"; | ||
DetectEngineCtx *de_ctx = DetectEngineCtxInit(); | ||
if (de_ctx == NULL) | ||
goto end; | ||
FAIL_IF(de_ctx == NULL); | ||
|
||
sig = SigInit(de_ctx, "alert tcp any any -> any any (msg:\"msg escape tests \\w\\x\\y\\'\\\"\\\\;\\:\"; flow:to_server,established; content:\"blah\"; uricontent:\"/blah/\"; sid: 100;)"); | ||
if(sig == NULL) | ||
goto end; | ||
FAIL_IF(sig == NULL); | ||
|
||
if (strcmp(sig->msg, teststringparsed) != 0) { | ||
printf("got \"%s\", expected: \"%s\": ",sig->msg, teststringparsed); | ||
goto end; | ||
} | ||
FAIL_IF(strcmp(sig->msg, teststringparsed) != 0); | ||
|
||
result = 1; | ||
end: | ||
if (sig != NULL) | ||
SigFree(de_ctx, sig); | ||
if (de_ctx != NULL) | ||
DetectEngineCtxFree(de_ctx); | ||
return result; | ||
PASS; | ||
} | ||
|
||
static int DetectMsgParseTest03(void) | ||
{ | ||
int result = 0; | ||
Signature *sig = NULL; | ||
const char *teststringparsed = "flow stateless to_server"; | ||
DetectEngineCtx *de_ctx = DetectEngineCtxInit(); | ||
if (de_ctx == NULL) | ||
goto end; | ||
FAIL_IF(de_ctx == NULL); | ||
|
||
FILE *fd = SCClassConfGenerateValidDummyClassConfigFD01(); | ||
SCClassConfLoadClassficationConfigFile(de_ctx, fd); | ||
|
||
sig = SigInit(de_ctx, "alert tcp any any -> any any (msg: \"flow stateless to_server\"; flow:stateless,to_server; content:\"flowstatelesscheck\"; classtype:bad-unknown; sid: 40000002; rev: 1;)"); | ||
if(sig == NULL) | ||
goto end; | ||
FAIL_IF(sig == NULL); | ||
|
||
if (strcmp(sig->msg, teststringparsed) != 0) { | ||
printf("got \"%s\", expected: \"%s\": ", sig->msg, teststringparsed); | ||
goto end; | ||
} | ||
FAIL_IF(strcmp(sig->msg, teststringparsed) != 0); | ||
|
||
result = 1; | ||
end: | ||
if (sig != NULL) | ||
SigFree(de_ctx, sig); | ||
if (de_ctx != NULL) | ||
DetectEngineCtxFree(de_ctx); | ||
return result; | ||
PASS; | ||
} | ||
|
||
/** | ||
|
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.
We have a slightly cleaner macro for checking pointers:
FAIL_IF_NULL(ptr)
.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.
Understood! I saw that option at first, but wasn't really sure it was ready for using, because of the issue open that seems to be related to that...