<p><a href="https://gerrit.osmocom.org/13137">View Change</a></p><p>23 comments:</p><ul style="list-style: none; padding: 0;"><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c">File src/libmsc/gsm_04_08.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/13137/9/src/libmsc/gsm_04_08.c@156">Patch Set #9, Line 156:</a> <code style="font-family:monospace,monospace">DEBUGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">How about LOG_MSC_A_CAT() here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@164">Patch Set #9, Line 164:</a> <code style="font-family:monospace,monospace">DEBUGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">How about LOG_MSC_A_CAT() here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@201">Patch Set #9, Line 201:</a> <code style="font-family:monospace,monospace">LOGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">How about LOG_MSC_A_CAT() here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@206">Patch Set #9, Line 206:</a> <code style="font-family:monospace,monospace">DEBUGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">How about LOG_MSC_A_CAT() here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@279">Patch Set #9, Line 279:</a> <code style="font-family:monospace,monospace">switch</code></p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">if (gsm48_hdr_msg_type(gh) != GSM48_MT_RR_PAG_RESP)<br> return -1;</pre></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@409">Patch Set #9, Line 409:</a> <code style="font-family:monospace,monospace"> msc_a_put(msc_a, __func__);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Shouldn't we also msc_a_put(MSC_A_USE_LOCATION_UPDATING) here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@416">Patch Set #9, Line 416:</a> <code style="font-family:monospace,monospace"> msc_a_put(msc_a, __func__);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Shouldn't we also msc_a_put(MSC_A_USE_LOCATION_UPDATING) here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@615">Patch Set #9, Line 615:</a> <code style="font-family:monospace,monospace">DEBUGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What about LOG_MSC_A() here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@617">Patch Set #9, Line 617:</a> <code style="font-family:monospace,monospace">DEBUGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What about LOG_MSC_A() here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@677">Patch Set #9, Line 677:</a> <code style="font-family:monospace,monospace"> gsm48_mi_type_name(mi_type), mi_string);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Still unaligned: one space is missing.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@719">Patch Set #9, Line 719:</a> <code style="font-family:monospace,monospace"> msg->data_len, sizeof(struct gsm48_service_request*));</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Fancy alignment needs one more space ;)</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@719">Patch Set #9, Line 719:</a> <code style="font-family:monospace,monospace">sizeof(struct gsm48_service_request*)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Wait, sizeof pointer?!? Looks like a bug.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@808">Patch Set #9, Line 808:</a> <code style="font-family:monospace,monospace">DEBUGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What about LOG_MSC_A() here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@825">Patch Set #9, Line 825:</a> <code style="font-family:monospace,monospace">DEBUGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What about LOG_MSC_A() here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@850">Patch Set #9, Line 850:</a> <code style="font-family:monospace,monospace"></code></p><p style="white-space: pre-wrap; word-wrap: break-word;">ws</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@852">Patch Set #9, Line 852:</a> <code style="font-family:monospace,monospace">LOGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What about LOG_MSC_A() here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@855">Patch Set #9, Line 855:</a> <code style="font-family:monospace,monospace">LOGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What about LOG_MSC_A() here?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@890">Patch Set #9, Line 890:</a> <code style="font-family:monospace,monospace"> msgb_l3len(msg));</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Broken alignment: +1 space.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@921">Patch Set #9, Line 921:</a> <code style="font-family:monospace,monospace">msgb_l3len(msg)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think it should be data_len.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@969">Patch Set #9, Line 969:</a> <code style="font-family:monospace,monospace"> "MM AUTHENTICATION RESPONSE: invalid: parsing %s AKA Auth Response"</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Broken alignment.</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@1202">Patch Set #9, Line 1202:</a> <code style="font-family:monospace,monospace">DEBUGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What about LOG_MSC_A()?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@1244">Patch Set #9, Line 1244:</a> <code style="font-family:monospace,monospace">DEBUGP</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">What about LOG_MSC_A()?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/gsm_04_08.c@1302">Patch Set #9, Line 1302:</a> <code style="font-family:monospace,monospace">cm_service_type</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Unused?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/13137">change 13137</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/13137"/><meta itemprop="name" content="View Change"/></div></div>
<div style="display:none"> Gerrit-Project: osmo-msc </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: I27e4988e0371808b512c757d2b52ada1615067bd </div>
<div style="display:none"> Gerrit-Change-Number: 13137 </div>
<div style="display:none"> Gerrit-PatchSet: 9 </div>
<div style="display:none"> Gerrit-Owner: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Vadim Yanitskiy <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-CC: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-CC: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Tue, 07 May 2019 22:22:16 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>