Skip to content

[WIP] - Varnish matching hardened#7

Open
jbarritaud wants to merge 7 commits into
masterfrom
varnish-matching-hardened
Open

[WIP] - Varnish matching hardened#7
jbarritaud wants to merge 7 commits into
masterfrom
varnish-matching-hardened

Conversation

@jbarritaud

Copy link
Copy Markdown
Contributor

NOT TESTED YET

Comment thread lib/modules/varnish/poll.c Outdated
struct timeval clock;

gettimeofday(&clock, NULL);
output_start = output;

Copy link
Copy Markdown
Member

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

Comment thread 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);

Copy link
Copy Markdown
Member

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.

Comment thread 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)) {

Copy link
Copy Markdown
Member

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)

Comment thread lib/modules/varnish/poll.c Outdated
Comment on lines +92 to +94
} else {
output = &found[strlen(metrics[i].varnish_name)]
}

Copy link
Copy Markdown
Member

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).

Comment thread lib/modules/varnish/poll.c Outdated
} else {
output = &found[strlen(metrics[i].varnish_name)]
}
if (strlen(output) < strlen(metrics[i].varnish_name)) {

Copy link
Copy Markdown
Member

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants