<p><a href="https://gerrit.osmocom.org/13137">View Change</a></p><p>7 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/cell_id_list.c">File src/libmsc/cell_id_list.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/cell_id_list.c@35">Patch Set #9, Line 35:</a> <code style="font-family:monospace,monospace">e->cell_id = *cid;</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Coverity won't be happy. Please OSMO_ASSERT(e) before referencing.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/e_link.c">File src/libmsc/e_link.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/e_link.c@77">Patch Set #9, Line 77:</a> <code style="font-family:monospace,monospace">*e = (struct e_link) {</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think you're abusing this way of structure initialization here. All fields of 'e', excluding 'gcm', are zero-initialized, but a few lines below you (re)initialize them again. This would also suppress the compiler's -Wuninitialized warnings. Rather do 'e->gcm = gcm' below.</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/e_link.c@84">Patch Set #9, Line 84:</a> <code style="font-family:monospace,monospace">memcpy</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">If 'remote_name' were of type 'const char *', you could just use osmo_strdup(). Why do we need 'uint8_t *'?</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/e_link.c@84">Patch Set #9, Line 84:</a> <code style="font-family:monospace,monospace">       memcpy(e->remote_name, remote_name, remote_name_len);</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">OSMO_ASSERT(e->remote_name) before referencing?</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/e_link.c@105">Patch Set #9, Line 105:</a> <code style="font-family:monospace,monospace">enum msc_role from_role</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Unused parameter?</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/e_link.c@127">Patch Set #9, Line 127:</a> <code style="font-family:monospace,monospace">strlen(local_msc_name)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">Do we need to also include '\0'?</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/e_link.c@132">Patch Set #9, Line 132:</a> <code style="font-family:monospace,monospace">if (vsub)</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">AFAIR, IMSI is mandatory for all GSUP messages. If 'vsub' is NULL, this function would prepare an incomplete GSUP message.</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 21:47:04 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>