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

fix #1569 - improve monitor_regex #1595

Merged
merged 2 commits into from Apr 8, 2021
Merged

fix #1569 - improve monitor_regex #1595

merged 2 commits into from Apr 8, 2021

Conversation

leibale
Copy link
Collaborator

@leibale leibale commented Apr 8, 2021

No description provided.

@leibale leibale requested a review from gkorland April 8, 2021 17:16
@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2021

This pull request fixes 1 alert when merging 79d4b26 into 7e77de8 - view on LGTM.com

fixed alerts:

  • 1 for Inefficient regular expression

@lgtm-com
Copy link

lgtm-com bot commented Apr 8, 2021

This pull request fixes 1 alert when merging 8c5506d into 7e77de8 - view on LGTM.com

fixed alerts:

  • 1 for Inefficient regular expression

@leibale leibale merged commit 2d11b6d into master Apr 8, 2021
@leibale leibale deleted the GHSL-2021-026 branch April 8, 2021 22:04
Copy link

@OnlineCop OnlineCop left a comment

Choose a reason for hiding this comment

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

The original, ending in ( ".+?")+$, would fail to match:

1234567890.0 [1 2]  "test"

(There are two spaces between the closing square bracket and the double quote.)

The new, ending in .*"$, will match that string (or any other string, so long as the last character on the line is a double quote).

Should this text match, or fail to match?

@leibale
Copy link
Collaborator Author

leibale commented Apr 28, 2021

@OnlineCop Basically you're right, but since this regex is used only to check if a message from Redis is a monitor message or a reply to a command, and the only message that starts with a decimal number is a reply from the monitor command, even the new regex is an overkill...

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.

None yet

3 participants