<p><a href="https://gerrit.osmocom.org/9211">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/9211/2/src/libvlr/vlr.c">File src/libvlr/vlr.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/9211/2/src/libvlr/vlr.c@469">Patch Set #2, Line 469:</a> <code style="font-family:monospace,monospace">     /* Table 10.5.33: The T3212 timeout value is coded as the</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">This comment was moved here, I didn't write it. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">no no, I was just mocking about the design from 3GPP. But the comment is actually helpful. Let's just fix the spec reference. I can find half of it at 3GPP TS 24.008 Table 10.5.1.12.2, and 3GPP TS 44.018 Table 10.5.2.11.1; but so far can't find this "Table 10.5.33" nor the spec to use twice T3212 plus 1 minute... can anyone else find it?</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/9211/2/src/libvlr/vlr.c@1144">Patch Set #2, Line 1144:</a> <code style="font-family:monospace,monospace">/* See TS 32.012 version 9.10.0 4.3.2.1 "Process Detach_IMSI_VLR" */</code></p><p style="white-space: pre-wrap; word-wrap: break-word;">23.012 !</p></li><li style="margin: 0; padding: 0 0 0 16px;"><p style="margin-bottom: 4px;"><a href="https://gerrit.osmocom.org/#/c/9211/2/src/libvlr/vlr.c@1192">Patch Set #2, Line 1192:</a> <code style="font-family:monospace,monospace">     osmo_timer_setup(&vlr->lu_expire_timer, vlr_subscr_expire_lu, vlr);</code></p><p><blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;">It's not in the same function. […]</blockquote></p><p style="white-space: pre-wrap; word-wrap: break-word;">yes, I saw that, but it doesn't make sense to setup the timer if it isn't started, so might as well setup in vlr_start() as well. Usually when I read code, I see a timer being started with the related callback function referenced right next to it. Makes it harder to read if that is spread about. So it's a cosmetic preference. Any hard reason to need this in vlr_alloc()?</p></li></ul></li></ul><p>To view, visit <a href="https://gerrit.osmocom.org/9211">change 9211</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/9211"/><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: Iebdee8b12d22acfcfb265ee41e71cfc8d9eb3ba9 </div>
<div style="display:none"> Gerrit-Change-Number: 9211 </div>
<div style="display:none"> Gerrit-PatchSet: 2 </div>
<div style="display:none"> Gerrit-Owner: Stefan Sperling <ssperling@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Harald Welte <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: Neels Hofmeyr <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Stefan Sperling <ssperling@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Fri, 18 May 2018 12:47:53 +0000 </div>
<div style="display:none"> Gerrit-HasComments: Yes </div>
<div style="display:none"> Gerrit-HasLabels: No </div>