<p style="white-space: pre-wrap; word-wrap: break-word;">The rest should be addressed in next revision.</p><p><a href="https://gerrit.osmocom.org/11827">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/11827/9/src/gsm/gsm29_205.c">File src/gsm/gsm29_205.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/11827/9/src/gsm/gsm29_205.c@82">Patch Set #9, Line 82:</a> <code style="font-family:monospace,monospace">  parsed += (2 + 1);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">parenthesis can be dropped here.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think it would reduce readability so I'd rather keep them.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11827/9/src/gsm/libosmogsm.map">File src/gsm/libosmogsm.map:</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/11827/9/src/gsm/libosmogsm.map@147">Patch Set #9, Line 147:</a> <code style="font-family:monospace,monospace">gsm29205_enc_gcr;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I know not everything is perfectly sorted, but let's try to keep it as sorted as possible, so move t […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Curious, why it should be sorted at all? I thought this file is loaded completely by linker and entries order does not matter.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/11827/9/tests/gsm29205/gsm29205_test.c">File tests/gsm29205/gsm29205_test.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/11827/9/tests/gsm29205/gsm29205_test.c@64">Patch Set #9, Line 64:</a> <code style="font-family:monospace,monospace">           printf("decoding failed: %s [%s]\n", strerror(-rc), msgb_hexdump(msg));</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">All these should exit, or at least return.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">Why? Printing unexpected text on the screen will cause test failure just like unexpected exit. At the same time running till the end of the test allows us to see multiple errors at once.</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/11827">change 11827</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/11827"/><meta itemprop="name" content="View Change"/></div></div>

<div style="display:none"> Gerrit-Project: libosmocore </div>
<div style="display:none"> Gerrit-Branch: master </div>
<div style="display:none"> Gerrit-MessageType: comment </div>
<div style="display:none"> Gerrit-Change-Id: Iee95aa4e5c056645b6cb5667e4a067097d52dfbf </div>
<div style="display:none"> Gerrit-Change-Number: 11827 </div>
<div style="display:none"> Gerrit-PatchSet: 9 </div>
<div style="display:none"> Gerrit-Owner: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder (1000002) </div>
<div style="display:none"> Gerrit-Reviewer: Max <msuraev@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Pau Espin Pedrol <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 30 Nov 2018 14:57:39 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>