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

issue 1486: better error message in likely missing semicolon - v2 #2260

Closed

Conversation

jasonish
Copy link
Member

@jasonish jasonish commented Sep 21, 2016

Attempt to detect missing semicolons by detecting unbalanced double quotes. Can give a more meaningful error message.

Redmine issue: https://redmine.openinfosecfoundation.org/issues/1486

Final commit includes conversion to unit test macros. I couldn't resist after having to fix up one test.

Prscript:

If a rule option value starts with a double quote, ensure it
ends with a double quote, exclusive of white space which gets
trimmed anyways.

Catches errors like 'filemagic:"picture" sid:5555555;' reporting
that a missing semicolon may be the error.
break;
}
}
if (optvalue[ovlen - 1] != '"') {
Copy link
Contributor

Choose a reason for hiding this comment

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

detect-parse.c: In function ‘SigParse’:
detect-parse.c:624:25: error: array subscript is above array bounds [-Werror=array-bounds]
             if (optvalue[ovlen - 1] != '"') {
                         ^

@jasonish
Copy link
Member Author

Trying to get the warning myself (not that there isn't a problem, I just want to make sure the error goes away as well). I'm using gcc 6.1.1 and clang 3.9.0, with -Werror=array-bounds. What are you using?

@inliniac
Copy link
Contributor

On 22-09-16 16:09, Jason Ish wrote:

Trying to get the warning myself (not that there isn't a problem, I just
want to make sure the error goes away as well). I'm using gcc 6.1.1 and
clang 3.9.0, with -Werror=array-bounds. What are you using?

This was on a Debian box, think it's clang 3.5. Saw it with gcc
4.9.2 as well.

@jasonish
Copy link
Member Author

Bounds fixed in new PR: #2265

@jasonish jasonish closed this Sep 22, 2016
@inliniac inliniac mentioned this pull request Sep 22, 2016
@inliniac
Copy link
Contributor

I merged the pcre cleanup commits in #2266, thanks Jason.

@jasonish jasonish deleted the ish-issue-1486-missing-semicolon-v2 branch September 26, 2016 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
2 participants