<p style="white-space: pre-wrap; word-wrap: break-word;">Please fix the cosmetic stuff and clarify the issue, then you have +2.</p><p><a href="https://gerrit.osmocom.org/c/osmo-ggsn/+/20534">View Change</a></p><p>3 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-ggsn/+/20534/2//COMMIT_MSG">Commit Message:</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/osmo-ggsn/+/20534/2//COMMIT_MSG@9">Patch Set #2, Line 9:</a> <code style="font-family:monospace,monospace">Don't compare EOF with 'true' which resulted in calling</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">This is confusing.<br>The crash doesn't come from comparing EOF as true, since EOF is actually defined as -1 here, so that's fine. The problem is the first call you modified which was checked wrongly (0 on success/found, EOF on not found)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/c/osmo-ggsn/+/20534/2/ggsn/ggsn_vty.c">File ggsn/ggsn_vty.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/osmo-ggsn/+/20534/2/ggsn/ggsn_vty.c@917">Patch Set #2, Line 917:</a> <code style="font-family:monospace,monospace">              if (0 == gtp_pdp_getimsi(ggsn->gsn, &pdp, imsi, nsapi)) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">rather compare against value on the right side, it's usually done like that since it's less confusing as if it was an assignment:<br>gtp_pdp_getimsi(ggsn->gsn, &pdp, imsi, nsapi) == 0</p><p style="white-space: pre-wrap; word-wrap: break-word;">Also drop the comment, no need for that.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/c/osmo-ggsn/+/20534/2/ggsn/ggsn_vty.c@924">Patch Set #2, Line 924:</a> <code style="font-family:monospace,monospace">                  if (EOF == gtp_pdp_getimsi(ggsn->gsn, &pdp, imsi, nsapi))</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Same as above.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-ggsn/+/20534">change 20534</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/c/osmo-ggsn/+/20534"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-ggsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-Change-Id: Ic40429939b185f97c020dd3904e054fe860b91e8 </div>
<div style="display:none"> Gerrit-Change-Number: 20534 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: keith <keith@rhizomatica.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Sun, 11 Oct 2020 21:52:02 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>