<p><a href="https://gerrit.osmocom.org/9474">View Change</a></p><p>2 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/9474/1/src/libosmo-mgcp/mgcp_protocol.c">File src/libosmo-mgcp/mgcp_protocol.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.osmocom.org/#/c/9474/1/src/libosmo-mgcp/mgcp_protocol.c@406">Patch Set #1, Line 406:</a> <code style="font-family:monospace,monospace">     if (!ptr)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't understand this code, sorry.</p><ul><li>We first get a pointer to the [first] ":"</li><li>we then enter the while loop, where we</li><li>* check if ptr <= options (which is not true, unless the ":" was the first character)</li><li>* we increment ptr, if *ptr is a space or a comma</li></ul><p style="white-space: pre-wrap; word-wrap: break-word;">But that increment could never happen, as we only ever enter the while loop with *ptr == ':'</p><p style="white-space: pre-wrap; word-wrap: break-word;">What am I missing?</p><p style="white-space: pre-wrap; word-wrap: break-word;">btw: these kind of side-effect free functions are the *ideal* candidates for unit tests inside the tests/ subdirectory.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/9474/1/src/libosmo-mgcp/mgcp_protocol.c@428">Patch Set #1, Line 428:</a> <code style="font-family:monospace,monospace">int check_local_cx_options(void *ctx, const char *options)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">this function should have unit test coverage with a variety of legal and illegal strings as input.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/9474">change 9474</a>. To unsubscribe, or for help writing mail filters, visit <a href="https://gerrit.osmocom.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.osmocom.org/9474"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-mgw </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Iae2fddfa5f2bcfc952f8ab217b3056694e5f7812 </div>
<div style="display:none"> Gerrit-Change-Number: 9474 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-CC: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 06 Jun 2018 12:21:40 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>