[Asterisk-code-review] StatsD: Add sample rate compatibility (asterisk[master])

[Tyler Cambron]

Tyler Cambron has posted comments on this change.

Change subject: StatsD: Add sample rate compatibility
......................................................................


Patch Set 1:

(1 comment)

https://gerrit.asterisk.org/#/c/1571/1/apps/app_statsd.c
File apps/app_statsd.c:

Line 130: 	for (i = 0; i < ARRAY_LEN(valid_metrics); i++) {
        : 		if (!strcmp(valid_metrics[i], metric)) {
        : 			return 0;
        : 		}
        : 	}
> I don't understand why this logic is sufficient. Consider the following, if
strcmp does not look to see if 'g', 's', 'c', or 'ms' are inside of the metric name, but it sees if each spot in the string match exactly what was given by the user. I ran the app with 'groovygeeks' as the metric type and it failed saying that 'groovygeeks' is not a valid metric type. It also failed with 'ggg' as the metric.

I could be missing other key concepts about the functionality of strcmp, but strstr is the function that checks to see if one string is contained in the other, I think.


-- 
To view, visit https://gerrit.asterisk.org/1571
To unsubscribe, visit https://gerrit.asterisk.org/settings

Gerrit-MessageType: comment
Gerrit-Change-Id: I11d315d0a5034fffeae1178e650aa8264485ed52
Gerrit-PatchSet: 1
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Tyler Cambron <tcambron at digium.com>
Gerrit-Reviewer: Ashley Sanders <asanders at digium.com>
Gerrit-Reviewer: Kevin Harwell <kharwell at digium.com>
Gerrit-Reviewer: Tyler Cambron <tcambron at digium.com>
Gerrit-HasComments: Yes