<p style="white-space: pre-wrap; word-wrap: break-word;">-1 because of missing breaks</p><p>Patch set 2:<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.osmocom.org/c/osmo-sgsn/+/12009">View Change</a></p><p>4 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/12009/2/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/12009/2/src/gprs/gprs_gmm.c@150">Patch Set #2, Line 150:</a> <code style="font-family:monospace,monospace">                  mmctx_set_mm_state(mm, MM_STANDBY);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">missing "break"</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12009/2/src/gprs/gprs_gmm.c@153">Patch Set #2, Line 153:</a> <code style="font-family:monospace,monospace">                             get_value_string(gprs_pmm_state_names, mm->pmm_state));</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">missing "break"</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12009/2/src/gprs/gprs_gmm.c@169">Patch Set #2, Line 169:</a> <code style="font-family:monospace,monospace">                    T, mm->gb.state_T);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Is it intentional, that the timer gets re-scheduled below even if the error message was written? If so, maybe it makes sense to mention this in the message, reading it now makes it seem like it does not change the timer, because it is already active.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/12009/2/src/gprs/gprs_gmm.c@3076">Patch Set #2, Line 3076:</a> <code style="font-family:monospace,monospace">           gsm0408_gprs_notify_pdu_gb(mmctx);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">why is this not part of the above "if (mmctx)"?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-sgsn/+/12009">change 12009</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-sgsn/+/12009"/><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-Change-Id: I4ce23ebe50d141076c20c9c56990b7103cd25e55 </div>
<div style="display:none"> Gerrit-Change-Number: 12009 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: laforge <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Assignee: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: lynxis lazus <lynxis@fe80.eu> </div>
<div style="display:none"> Gerrit-Reviewer: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 20 Aug 2019 11:07:02 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-Has-Labels: Yes </div>
<div style="display:none"> Gerrit-MessageType: comment </div>