Quick Audit Notes for https://github.com/grobian/carbon-c-relay Jon Simons 3/23/2015
--
Looks mostly okay. There are a number of unchecked mallocs and strdups throughout but
it's not clear whether with our memory profiles in production these spots would actually
result in real-life problems. We can fix them. There was at least one spot in the code
using usleep as a pre-cleanup step for some of its state: that spot is likely incorrect
code.
--
PRs and Issues don't seem to show any outright red flags from what I can tell:
Open Issues (worth watching and further vetting):
- unsolved "gaps in aggregates" issue; not seen by maintainer grobian/carbon-c-relay#33
- maybe wrong timestamp computations grobian/carbon-c-relay#35
- unsolved segfault on CentOS grobian/carbon-c-relay#46
Open PRs (just one):
- for a clang-ism: grobian/carbon-c-relay#53.
Open Issues (minor):
- FR for hot-reload route configs grobian/carbon-c-relay#32
- config question (looks like no bug) grobian/carbon-c-relay#41
- FR for CPU and memory stats grobian/carbon-c-relay#45
- FR for unconditionally
tolowering stat names grobian/carbon-c-relay#48 - garbled metrics paths; solved (?) on latest version grobian/carbon-c-relay#52
--
Source: 5700 LoC; Apache license. Skimmed over looking for obvious superficial problems and none
seemed too drastic. There is some gnarly parsing code but perhaps using it would not be too
bad assuming we have repro cases if/when we hit bugs. I'd be most cautious about the regex routing
engine. I'd probably ditch select just to never have to think about hitting FD_SETSIZE issues
but I'm not sure if that would be a real possibility in this context.
- aggregator.c unchecked
malloc,strdup - router.c unchecked
mallochere and throughout; though this looks like init-time setup maybe not steady-state paths - aggregator.c 250ms main loop sleep
- dispatcher.c Shady cleanup-path
- dispatcher.c Beware FD_SETSIZE (not sure if a real problem in this context)
- router.c Think Carefully about Regex Routing Rules
- router.c How Does This Fare With Lots of "foo.bar.value"
- router.c Beware Unbounded Recursion on User-Input (I'd prefer this were bounded) (this might be bounded to number of routes in config file; not sure at a glance)