-
Notifications
You must be signed in to change notification settings - Fork 1
[WIP] - Varnish matching hardened #7
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
base: master
Are you sure you want to change the base?
Conversation
lib/modules/varnish/poll.c
Outdated
| struct timeval clock; | ||
|
|
||
| gettimeofday(&clock, NULL); | ||
| output_start = output; |
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.
reuse the "found" variable instead (reset it above the while to output and do found += strlen(metrics[i].varnish_name) instead of output = &found[strlen(metrics[i].varnish_name)]
Also a for loop would be good, see below
lib/modules/varnish/poll.c
Outdated
| VARNISH_SMA_TRANSIENT_LEN)) | ||
| && (isspace(found[strlen(metrics[i].varnish_name)]))) { | ||
| char *endval; | ||
| char *val = found + strlen(metrics[i].name); |
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.
It should be char *val = found + strlen(metrics[i].varnish_name); here, don't know how it worked before, probably padding or something saved us because strtoull ignore leading spaces.
lib/modules/varnish/poll.c
Outdated
| sprintf(metric_name, "%s", metrics[i].name); | ||
| n_metrics += monikor_metric_push(mod->mon, monikor_metric_integer( | ||
| metric_name, &clock, value, metrics[i].flags)); | ||
| while (found = strstr(output, metrics[i].varnish_name)) { |
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.
I'd do for (found = output; (found = strstr(found, metrics[i].varnish_name)); found += strlen(metrics[i].varnish_name)) instead
Please note the extra parenthesis around (found = ... which were missing on the while (thus always evaluating to true because an assignment is always true)
lib/modules/varnish/poll.c
Outdated
| } else { | ||
| output = &found[strlen(metrics[i].varnish_name)] | ||
| } |
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.
Don't need the else because there is a break, putting output = ... below would be sufficient (see comment above to replace the thing with a for loop anyways).
lib/modules/varnish/poll.c
Outdated
| } else { | ||
| output = &found[strlen(metrics[i].varnish_name)] | ||
| } | ||
| if (strlen(output) < strlen(metrics[i].varnish_name)) { |
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.
This is not necessary either because if strstr returned non-null we know there is at least strlen(metrics[i].varnish_name) characters remaining so it's safe to increment the pointer and the loop will just stop by itself once all matches were processed
NOT TESTED YET