<p><a href="https://gerrit.osmocom.org/13744">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/13744/7/src/gprs/gprs_gmm.c">File src/gprs/gprs_gmm.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/13744/7/src/gprs/gprs_gmm.c@1618">Patch Set #7, Line 1618:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;"><br>bool pdp_status_has_no_active_nsapis(const uint8_t *pdp_status, const size_t pdp_status_len)<br>{<br>       size_t i;<br>     for (i = 0; i < pdp_status_len; i++)<br>               if (pdp_status[i] != 0)<br>                       return false;<br><br>       return true;<br>}<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Can you rename it to pdp_status_has_active_nsapis?<br>It would also mean you have to swap the logic. A positive check is more readable as a negation check.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13744/7/src/gprs/gprs_gmm.c@1629">Patch Set #7, Line 1629:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">/* 3GPP TS 24.008 Section 4.7.13.4 Service request procedure not accepted by the<br> * network. Returns true if MS has active PDP contexts in pdp_status but SGSN<br> * has none */<br>bool ms_has_active_pdp_and_sgsn_has_none(struct sgsn_mm_ctx *mmctx,<br>                                       const uint8_t *pdp_status,<br>                                    const size_t pdp_status_len)<br>{<br>      if (!llist_empty(&mmctx->pdp_list))<br>            return false;<br><br>       return !pdp_status_has_no_active_nsapis(pdp_status, pdp_status_len);<br>}<br></pre></blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Can you remove this function. Doing another redirection of a function just for this single case seems to be confusing.<br>Or do you plan do add this check more often? If so I would like to rename this function to check_pdp_status()</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13744/7/src/gprs/gprs_gmm.c@1935">Patch Set #7, Line 1935:</a> <code style="font-family:monospace,monospace">ms_has_active_pdp_and_sgsn_has_none(ctx, pdp_status, pdp_status_len)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">do a llist_empty(..) && pdp_status_has_active_nsapis()</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/13744">change 13744</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/13744"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: osmo-sgsn </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: If610cbef17c25ec44e65d4f1b2340d102c560437 </div>
<div style="display:none"> Gerrit-Change-Number: 13744 </div>
<div style="display:none"> Gerrit-PatchSet: 7 </div>
<div style="display:none"> Gerrit-Owner: Mykola Shchetinin <mykola@pentonet.com> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Mykola Shchetinin <mykola@pentonet.com> </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 08 May 2019 13:00:26 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>