Skip to content

Refactoring conditional directives. #996

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

Closed
wants to merge 4 commits into from
Closed

Refactoring conditional directives. #996

wants to merge 4 commits into from

Conversation

wmarquesr
Copy link

A suggestion to compile entire statements and expressions, as suggested by code style guidelines from the Linux Kernel and practitioners.

It might improve code understanding, maintainability and error-proneness.

@csanders-git
Copy link

I'll leave this for @zimmerle but you should check out the the libmodsecurity branch as it has been rewritten with a more logical design and stricter coding standards defined.

@zimmerle
Copy link
Contributor

zimmerle commented Dec 9, 2015

Hi @wmarquesr,

Thank you for the patch. The code is currently in our BuildBots.

Same modifications are more than welcomed on LibModSecurity, available here:
https://github.com/SpiderLabs/ModSecurity/tree/libmodsecurity

@zimmerle
Copy link
Contributor

zimmerle commented Dec 9, 2015

@wmarquesr
Copy link
Author

All fixed. Sorry for the mistake.

@zimmerle
Copy link
Contributor

Thank you @wmarquesr,

Patch is on our buildbots again.

@zimmerle
Copy link
Contributor

zimmerle commented Jan 4, 2016

Hi @wmarquesr,

Still interested to provide the fix ?

@wmarquesr
Copy link
Author

All fixed, sorry for delay.

@zimmerle
Copy link
Contributor

zimmerle commented Jan 5, 2016

Hi @wmarquesr,

Thank you! Your patch was merged and will be released as part of ModSecurity v2.9.1.

The same kind of patch are very welcomed in ModSecurity version 3:

@zimmerle zimmerle closed this Jan 5, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants