<blockquote style="border-left: 1px solid #aaa; margin: 10px 0; padding: 0 10px;"><p style="white-space: pre-wrap; word-wrap: break-word;">Patch Set 1: Code-Review-1</p><p style="white-space: pre-wrap; word-wrap: break-word;">(7 comments)</p><p style="white-space: pre-wrap; word-wrap: break-word;">All in all very nice.</p><p style="white-space: pre-wrap; word-wrap: break-word;">Please also add range tests to all of the *.vty transcript tests, see tests/tdef/tdef_vty*, so that we can also verify the VTY behavior.</p><p style="white-space: pre-wrap; word-wrap: break-word;">BTW, in order to benefit from the range checks, all tdef users need to be changed to use osmo_tdef_set() instead of osmo_tdef_get_entry()? Do you have related patches? ... just realizing that this only needs to be done as soon as an application actually sets min and max values, so that should be fine. But this should be mentioned in the API doc of the new min and max members.</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">Most tdef users actually set the values through VTY, and that one is already checking for ranges (done in this patch). I recently introduced osmo_tdef_set myself in osmo-pcu for timers whose values are coming from osmo-bts, so those are working already. I still need to check if there are other users os osmo_tdef setting tdef->val manually. But in any case it doesn't hurt because they don't have min_val or max_val set, so range check is not applied there.</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/15546">View Change</a></p><ul style="list-style: none; padding: 0;"></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/15546">change 15546</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/c/libosmocore/+/15546"/><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-Change-Id: I4661ac41c29a009a1d5fc57d87aaee6041c7d1b2 </div>
<div style="display:none"> Gerrit-Change-Number: 15546 </div>
<div style="display:none"> Gerrit-PatchSet: 1 </div>
<div style="display:none"> Gerrit-Owner: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: fixeria <axilirator@gmail.com> </div>
<div style="display:none"> Gerrit-Reviewer: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: pespin <pespin@sysmocom.de> </div>
<div style="display:none"> Gerrit-CC: laforge <laforge@gnumonks.org> </div>
<div style="display:none"> Gerrit-Comment-Date: Wed, 18 Sep 2019 10:49:29 +0000 </div>
<div style="display:none"> Gerrit-HasComments: No </div>
<div style="display:none"> Gerrit-Has-Labels: No </div>
<div style="display:none"> Gerrit-MessageType: comment </div>