<p style="white-space: pre-wrap; word-wrap: break-word;">hmm, my idea was to have received this review a lot earlier. I am N times through final-final and absolutely final testing after patch changes. I appreciate the review, but to be honest I personally need to get this load off my shoulders. I am applying your review thus far, but would prefer to not squash more modifications into this patch set, but rather move on to separate follow-up patches after this.</p><p style="white-space: pre-wrap; word-wrap: break-word;">This patch being in limbo is a huge burden:</p><ul><li>having to continuously rebase it keeps introducing merge conflicts that are completely loaded onto my shoulders -- I have to understand and adjust to all other work going on; this limbo has been going on for far too long and by now is profoundly impacting my private life (stress / frustration).</li></ul><ul><li>Secondly, squashing changes into the patch set happens sort of hidden, not seen in the git history; I've often had to revisit the reflog to analyse past versions of the patch set to fix bugs introduced by modifications squashed into this -- that's not how version control should happen.</li></ul><p style="white-space: pre-wrap; word-wrap: break-word;">So please, unless you have absolutely critical bugs that need fixing, let's move to merge + separate patches as soon as possible.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Nevertheless: many thanks for reviewing this with close scrutiny!<br>If you can, by all means please go on to do so!</p><p><a href="https://gerrit.osmocom.org/13137">View Change</a></p><p>28 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/include/osmocom/msc/Makefile.am">File include/osmocom/msc/Makefile.am:</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/include/osmocom/msc/Makefile.am@9">Patch Set #9, Line 9:</a> <code style="font-family:monospace,monospace">gsm_04_11.h \</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Looks like a meaningless change.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I think I decided to alphabetically sort (vim ':sort' command) ... can we just keep this?</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/13137/6/include/osmocom/msc/msc_roles.h">File include/osmocom/msc/msc_roles.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/13137/6/include/osmocom/msc/msc_roles.h@379">Patch Set #6, Line 379:</a> </p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><pre style="font-family: monospace,monospace; white-space: pre-wrap;">struct an_apdu {<br>   /* accessNetworkProtocolId */<br> enum osmo_gsup_access_network_protocol an_proto;<br>      /* signalInfo */<br>      struct msgb *msg;<br>     /* If this AN-APDU is sent between MSCs, additional information from the E-interface messaging, like the<br>       * Handover Number, will placed/available here. Otherwise may be left NULL. */<br>        const struct osmo_gsup_message *e_info;<br>};<br></pre></blockquote></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">hmm, indeed, didn't see that... […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">In the end decided against using the msgb->cb[], because it makes the type conversions a lot more complex / ugly.</p></li></ul></li><li style="margin: 0; padding: 0;"><p><a href="https://gerrit.osmocom.org/#/c/13137/9/src/libmsc/call_leg.c">File src/libmsc/call_leg.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/call_leg.c@34">Patch Set #9, Line 34:</a> <code style="font-family:monospace,monospace">LOG_CALL_LEG</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">LOGPFSML can handle fi=NULL, so we probably don't need this macro.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">indeed it ended up being a simplistic wrapper, but it is my preferred pattern to keep FSM specific log macros. That way we can trivially tweak all log context for a given object in a single place.</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/call_leg.c@78">Patch Set #9, Line 78:</a> <code style="font-family:monospace,monospace">fi</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">AFAICS, you're using 'fi' as the talloc-parent in order to deallocate it automatically when the 'fi' […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I want the struct and the fi to be one talloc unit, and I specifically do not want to have to remember to talloc_free in a cleanup function.</p><p style="white-space: pre-wrap; word-wrap: break-word;">It is also a functional requirement to keep the struct around until the FSM instance frees to avoid all sorts of complex freeing cascades hitting use-after-free problems. See http://git.osmocom.org/libosmocore/commit/?id=1f9cc018618e7d1e9a7a37e1ef08e059a4e02e87 -- which would be quite useless if the cleanup function freed the struct separately.</p><p style="white-space: pre-wrap; word-wrap: break-word;">You will see this exact pattern in *all* of the FSM instance implementations I have written, both in osmo-bsc and here. I appreciate your opinion, but will not adjust this pattern.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't understand what you mean by "no need to init dummy call_leg_fsm on line #59." -- line 59 has the global FSM definition, which is not the FSM instance that is allocated here based on that singleton FSM definition. Again this is a common pattern.</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/call_leg.c@78">Patch Set #9, Line 78:</a> <code style="font-family:monospace,monospace">talloc_zero</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Since you're doing *cl = (struct call_leg) { ... […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">you mean I don't need to talloc_zero()? Below cl = (struct...){...} can be seen as syntactic sugar to not have to repeat cl-> in every line, and to make clear that the struct is fully initialized.<br>The talloc API does not provide a macro similar to talloc_zero() that names the struct but doesn't zero initialize.<br>What is the reason for this statement, optimization? I would just keep it as is, no point in changing it?</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/call_leg.c@101">Patch Set #9, Line 101:</a> <code style="font-family:monospace,monospace">cl->fi</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Looks like a bug to me. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">osmo_fsm_inst_change_reparent() indeed contains this bug.<br>Thanks for highlighting, I found it while neck deep in developing and forgot about it.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Opened https://osmocom.org/issues/3983</p><p style="white-space: pre-wrap; word-wrap: break-word;">Since we need an osmo_fsm_inst_change_parent2() for compat, this code can be merged as-is now, and can be fixed once the new osmo_fsm_inst_change_parent2() exists.</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/call_leg.c@142">Patch Set #9, Line 142:</a> <code style="font-family:monospace,monospace"></code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">ws</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">the blank line is intentional</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/call_leg.c@145">Patch Set #9, Line 145:</a> <code style="font-family:monospace,monospace">for (i = 0; i < ARRAY_SIZE(cl->rtp); i++) {</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">So here we check whether all RTP connections of this call leg are established, right? Would be great […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">IMHO the code is quite clear, but ok.</p><p style="white-space: pre-wrap; word-wrap: break-word;">What you see as simplification is code duplication in my eyes. I prefer to traverse arrays in for loops, even if they have only two members.</p><p style="white-space: pre-wrap; word-wrap: break-word;">We don't suppress. Incoming event is an RTP stream saying it is ready, and if all are ready we pass on the *CALL_LEG* event that the entire call leg is established. No logging needed besides the events that are already logged.</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/call_leg.c@153">Patch Set #9, Line 153:</a> <code style="font-family:monospace,monospace">if (cl->fi->state != CALL_LEG_ST_ESTABLISHED)</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This check could be done before iterating over 'cl->rtp': […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I don't remember why I added this if()... theoretically, if any call leg is non-established, the call leg should also not be in established state.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I think I would rather remove the if() and have some error log when re-entering the same state...</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/call_leg.c@158">Patch Set #9, Line 158:</a> <code style="font-family:monospace,monospace">rtps = data;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This assignment looks useless to me. You could pass 'data' directly to osmo_fsm_inst_dispatch().</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">it is to clarify the type of the data argument. Otherwise the reader has to move to other parts of the code to investigate what the type is intended to be. It is a common pattern that indeed doesn't have much functional use in this case, yet I would prefer to keep it.</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/call_leg.c@178">Patch Set #9, Line 178:</a> <code style="font-family:monospace,monospace">struct call_leg *cl = fi->priv;</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Same here, just pass 'fi->priv' directly to osmo_fsm_inst_dispatch(). […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">same humble opinion, would prefer to keep for some degree of "type safety"</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/call_leg.c@211">Patch Set #9, Line 211:</a> <code style="font-family:monospace,monospace">{}</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I know that you prefer this way of array-termination, but in general we use { 0, NULL }. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I am being consistent with using {} everywhere in my patches all over the place for years :P<br>The cat is already out of the bag.</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/call_leg.c@229">Patch Set #9, Line 229:</a> <code style="font-family:monospace,monospace">call_leg_fsm_establishing_established</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">The function name looks confusing... […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">one function for both states</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/call_leg.c@244">Patch Set #9, Line 244:</a> <code style="font-family:monospace,monospace">/* same action function as above */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Ok, now I see. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">it is mostly for logging</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/call_leg.c@248">Patch Set #9, Line 248:</a> <code style="font-family:monospace,monospace">in_event_mask</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Since we terminate the FSM instance as soon as we enter this state, is there a chance that it would  […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">of course, cleanup and termination events ricocheting often dispatch numerous events</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/call_leg.c@253">Patch Set #9, Line 253:</a> <code style="font-family:monospace,monospace">/* same action function as above */</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Copy-pasted ;)</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">thx</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/call_leg.c@257">Patch Set #9, Line 257:</a> <code style="font-family:monospace,monospace">static struct osmo_fsm call_leg_fsm</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This symbol was already defined at line #59.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">it obviously has to be done exactly this way, unless I move the alloc function to below here. I prefer it in the beginning though. It's called "forward declaration" and is a common tool in the C language.</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/call_leg.c@290">Patch Set #9, Line 290:</a> <code style="font-family:monospace,monospace">call_leg_ensure_rtp_alloc</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Basically, this function just calls call_leg_rtp_alloc() and checks if the allocation was successful […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">the critical difference is that it calls call_leg_rtp_alloc() only if it hasn't been called yet on that rtp_stream. I would like to keep this.</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">I think you're abusing this way of structure initialization here. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">I prefer this to clearly indicate that the struct is initialized from scratch. You are right that it seems unnecessary, but nevertheless is a common patter I use everywhere:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">   foo = talloc<br>   *foo = (struct foo){<br>        initialze all members that can be initialized inline like this<br>   }</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">   initialize all members that need multiple lines and function calls, like memcpy and so forth</pre><p style="white-space: pre-wrap; word-wrap: break-word;">what's the point of changing this...</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">If 'remote_name' were of type 'const char *', you could just use osmo_strdup(). […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">it is a blob with length, to one day support Global Title or something. Wasn't my idea, but it is a BLOB, not a char*.</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Unused parameter?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">indeed, came from an earlier patch set where the role was reflected in the GSUP_ENTITY. Now we have just the GSUP_MESSAGE_CLASS</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Do we need to also include '\0'?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">yes :/ it's a long story...</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">AFAIR, IMSI is mandatory for all GSUP messages. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">the idea is that e_prep_gsup_msg() is generally working. If the result is an invalid message, tough luck for the caller, but I definitely want to avoid null pointer access. The caller might be aware of a missing vsub and obtain the IMSI otherwise? (not sure if any such caller exists, but that is the semantic idea of this function)</p></li></ul></li><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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">How about LOG_MSC_A_CAT() here?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">yes. This is legacy logging, not sure if we really need to change all of it now?<br>I agree that this can be changed but will not spend time on it.<br>Feel free to post a follow-up patch.</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><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">if (gsm48_hdr_msg_type(gh) != GSM48_MT_RR_PAG_RESP) […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">no: one day another message type comes along and we need to change to a switch() again.<br>Following same pattern as above.</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@409">Patch Set #9, Line 409:</a> <code style="font-family:monospace,monospace">              msc_a_put(msc_a, __func__);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Shouldn't we also msc_a_put(MSC_A_USE_LOCATION_UPDATING) here?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">the MSC_A_USE_LOCATION_UPDATING is put() by the FSM instance</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Wait, sizeof pointer?!? Looks like a bug.</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">hmmm</p><p style="white-space: pre-wrap; word-wrap: break-word;">this bug predates this patch, feel free to fix separately</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><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">Unused?</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">defined by vlr.ops API, even if we don't use it, we comply with that cb signature</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 23:39:56 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>