<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #ffd4d4;">Code-Review -1</span></p><p><a href="https://gerrit.asterisk.org/9361">View Change</a></p><p>7 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.asterisk.org/#/c/9361/1/main/test.c">File main/test.c:</a></p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9361/1/main/test.c@272">Patch Set #1, Line 272:</a> <code style="font-family:monospace,monospace">static char *reserved_words[] = {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">A comment saying which language's reserved words we are listing would be helpful.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9361/1/main/test.c@285">Patch Set #1, Line 285:</a> <code style="font-family:monospace,monospace">static int is_reserved_word(const char *word) {</code></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">Curly location.<br>int foo(void)<br>{<br> return 0;<br>}</pre></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9361/1/main/test.c@300">Patch Set #1, Line 300:</a> <code style="font-family:monospace,monospace"> char *tmp_cat = ast_strdupa(test->info.category + 1);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Instead of tmp_cat and tmp_name, how about test_cat and test_name?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9361/1/main/test.c@301">Patch Set #1, Line 301:</a> <code style="font-family:monospace,monospace"> char *next_cat = NULL;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Initializing to NULL is not necessary since the first use is initializing it.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9361/1/main/test.c@303">Patch Set #1, Line 303:</a> <code style="font-family:monospace,monospace"> struct ast_str *category = ast_str_create(strlen(test->info.category) + 32);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">category is never freed</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9361/1/main/test.c@305">Patch Set #1, Line 305:</a> <code style="font-family:monospace,monospace"> if (!category || !f || !test || test->state == AST_TEST_NOT_RUN) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">We are testing test for NULL after we have already dereferenced it when initializing the above variables.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.asterisk.org/#/c/9361/1/main/test.c@309">Patch Set #1, Line 309:</a> <code style="font-family:monospace,monospace"> while((next_cat = ast_strsep(&tmp_cat, '/', AST_STRSEP_TRIM))) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">space after wile</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.asterisk.org/9361">change 9361</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.asterisk.org/settings">settings</a>.</p><div itemscope itemtype="http://schema.org/EmailMessage"><div itemscope itemprop="action" itemtype="http://schema.org/ViewAction"><link itemprop="url" href="https://gerrit.asterisk.org/9361"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: asterisk </div>
<div style="display:none"> Gerrit-Branch: 13 </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Iec1a985eba1c478e5c1d65d5dfd95cb708442099 </div>
<div style="display:none"> Gerrit-Change-Number: 9361 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: George Joseph <gjoseph@digium.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins2 </div>
<div style="display:none"> Gerrit-Reviewer: Richard Mudgett <rmudgett@digium.com> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 06 Jul 2018 16:54:23 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: Yes </div>