<p>Patch set 1:<span style="border-radius: 3px; display: inline-block; margin: 0 2px; padding: 4px;background-color: #d4ffd4;">Code-Review +1</span></p><p><a href="https://gerrit.osmocom.org/c/osmo-sgsn/+/15211">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/15211/1/include/osmocom/sgsn/gprs_sgsn.h">File include/osmocom/sgsn/gprs_sgsn.h:</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/15211/1/include/osmocom/sgsn/gprs_sgsn.h@264">Patch Set #1, Line 264:</a> <code style="font-family:monospace,monospace">#define LOGIUP(level, ue, fmt, args...) \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(cosmetic)</p><p style="white-space: pre-wrap; word-wrap: break-word;">hmm, I see that LOGMMCTXP() also has level as first arg, but AFAIK all the rest of the Osmocom code base uses this order:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  LOGFOO(object, level, fmt, args...)</pre><p style="white-space: pre-wrap; word-wrap: break-word;">Probably not worth the effort, but I'd prefer new LOG macros to be consistent with that ordering, and maybe one day use sed to adjust LOGMMCTXP() to that order as well...?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/15211/1/include/osmocom/sgsn/gprs_sgsn.h@284">Patch Set #1, Line 284:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(in osmo-msc and -bsc I named the logging macros more like LOG_HO and LOG_MSC_A, for readability. Here that would be LOG_IU, LOG_GB... I'd prefer that but you decide.)</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/15211/1/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/15211/1/src/gprs/gprs_gmm.c@185">Patch Set #1, Line 185:</a> <code style="font-family:monospace,monospace">          LOGIUP(LOGL_NOTICE, ctx, "Cannot find mm ctx for IU event %d\n", type); \</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">(we have a value string for those events, see ranap_iu_event_type_str() from iu_client.h)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/15211/1/src/gprs/gprs_gmm.c@221">Patch Set #1, Line 221:</a> <code style="font-family:monospace,monospace">                   LOGIUP(LOGL_NOTICE, ctx, "Unknown event received: %i\n", type);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">ranap_iu_event_type_str()?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/osmo-sgsn/+/15211">change 15211</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/+/15211"/><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: Iba22060d8646bc8ec6227684ccb91d98cb4c7be2 </div>
<div style="display:none"> Gerrit-Change-Number: 15211 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 14 Aug 2019 23:52:46 +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>