<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 4: Code-Review+1</p><p style="white-space: pre-wrap; word-wrap: break-word;">What about uint8_t?</p></blockquote><p style="white-space: pre-wrap; word-wrap: break-word;">if you're asking for an osmo_str_to_uint8(), then it is a continuation of above discussion: should we add a separate function signature for each integer data type? My personal opinion is that we don't need those and should rather not have API creep for all integer types, given that literally the entire universe fits in an int64_t (signed or unsigned). I'd use constructs like this:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  int val;<br>  if (osmo_str_to_int(&val, "123", 10, 0, 255))<br>          return -EINVAL;<br>  my_obj->my_uint8 = val;<br>  return 0;</pre><p style="white-space: pre-wrap; word-wrap: break-word;">But two reviewers would prefer adding unsigned data types. My status is that I am ok with more API added at a later stage (probably by others), but I will not add more function signatures at this point.</p><p style="white-space: pre-wrap; word-wrap: break-word;">I'm having another idea, we could flip rc and val like this:</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  int64_t osmo_str_to_int64(int *rc, ...);</pre><p style="white-space: pre-wrap; word-wrap: break-word;">to get</p><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  int rc;<br>  my_obj->my_uint8 = osmo_str_to_int(&rc, "123", 10, 0, 255);<br>  if (rc)<br>        return -EINVAL;<br>  <br>I chose the other way because when the rc evaluation happens *after* assigning the value,<br>we need to either recover back, or anyway use a tmp val placeholder:</pre><pre style="font-family: monospace,monospace; white-space: pre-wrap;">  int rc;<br>  uint8_t val;<br>  val = osmo_str_to_int(&rc, "123", 10, 0, 255);<br>  if (rc)<br>        return -EINVAL;<br>  my_obj->my_uint8 = val;<br>  return 0;</pre><p style="white-space: pre-wrap; word-wrap: break-word;">So this ends up needing *more* local variables than when rc is returned directly (compare to first code snippet above).</p><p style="white-space: pre-wrap; word-wrap: break-word;">If you guys prefer the implicit cast to whichever int type we can also do that?<br>(But I guess that's not the point, is it??)</p><p><a href="https://gerrit.osmocom.org/c/libosmocore/+/25345">View Change</a></p><ul style="list-style: none; padding: 0;"></ul><p>To view, visit <a href="https://gerrit.osmocom.org/c/libosmocore/+/25345">change 25345</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/+/25345"/><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: I4dac826aab00bc1780a5258b6b55d34ce7d50c60 </div>
<div style="display:none"> Gerrit-Change-Number: 25345 </div>
<div style="display:none"> Gerrit-PatchSet: 4 </div>
<div style="display:none"> Gerrit-Owner: neels <nhofmeyr@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: Jenkins Builder </div>
<div style="display:none"> Gerrit-Reviewer: dexter <pmaier@sysmocom.de> </div>
<div style="display:none"> Gerrit-Reviewer: laforge <laforge@osmocom.org> </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: osmith <osmith@sysmocom.de> </div>
<div style="display:none"> Gerrit-Comment-Date: Mon, 13 Sep 2021 09:56:05 +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>