[Asterisk-code-review] StatsD: Add user input validation to the application (asterisk[master])

[Mark Michelson]

Mark Michelson has posted comments on this change.

Change subject: StatsD: Add user input validation to the application
......................................................................


Patch Set 4:

(4 comments)

https://gerrit.asterisk.org/#/c/1445/4/apps/app_statsd.c
File apps/app_statsd.c:

Line 71: static int validate_metric(char* metric)
Two things:

1) Coding guidelines say to place the * next to the variable name (char *metric instead of char* metric)
2) metric could be made const.


Line 80: 		for (i = 0; i < 4; i++) {
Asterisk provides a macro called ARRAY_LEN that works well for loops like this.

    for (i = 0; i < ARRAY_LEN(valid_metrics); i++)


Line 102: static int validate_name(char* name)
Like with validate_metric, the * needs to be moved and the parameter can be made const.


Line 122: static int validate_value(char* value)
Have a look at https://github.com/b/statsd_spec .

One thing that StatsD specifies is that different types of stats have different ranges of valid values. For instance, several types specify that negative values are unallowed. So it's probably a good idea to pass the metric and the value to this function so you can ensure that you do not allow an invalid value for certain metrics.

Also, like with validate_metric and validate_name, move the * over and make the parameter const.


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

Gerrit-MessageType: comment
Gerrit-Change-Id: I55c7ce44326a68ad6c5c1514b9575ac50f25bbc3
Gerrit-PatchSet: 4
Gerrit-Project: asterisk
Gerrit-Branch: master
Gerrit-Owner: Tyler Cambron <tcambron at digium.com>
Gerrit-Reviewer: Anonymous Coward #1000019
Gerrit-Reviewer: Mark Michelson <mmichelson at digium.com>
Gerrit-Reviewer: Matt Jordan <mjordan at digium.com>
Gerrit-HasComments: Yes