<p><b>@miri64</b> commented on this pull request.</p>

<hr>

<p>In <a href="https://github.com/RIOT-OS/RIOT/pull/11564#discussion_r305236025">sys/net/credman/credman.c</a>:</p>
<pre style='color:#555'>> +        goto end;
+    }
+    pos = _find_credential_pos(credential->tag, credential->type);
+    if (pos >= 0) {
+        DEBUG("credman: credential with tag %d and type %d already exist\n",
+              credential->tag, credential->type);
+        ret = CREDMAN_EXIST;
+        goto end;
+    }
+    /* find the next free position in credential pool */
+    pos = _find_credential_pos(CREDMAN_TAG_EMPTY, CREDMAN_TYPE_EMPTY);
+    if (pos < 0) {
+        DEBUG("credman: no space for new credential\n");
+        ret = CREDMAN_NO_SPACE;
+        goto end;
+    }
</pre>
<p>I can't find your comment on this in Github, but I found the following in my mails</p>
<blockquote>
<p>This is what I came up with based on suggestion from Cenk:</p>
<div class="highlight highlight-source-c"><pre><span class="pl-c"><span class="pl-c">/*</span> Find position of credential with given tag and type.</span>
<span class="pl-c"> * To avoid going through the list twice, this function also returns the</span>
<span class="pl-c"> * first empty position found.</span>
<span class="pl-c"> <span class="pl-c">*/</span></span>
<span class="pl-k">static</span> <span class="pl-k">int</span> <span class="pl-en">_find_credential_pos</span>(<span class="pl-c1">credman_tag_t</span> tag, <span class="pl-c1">credman_type_t</span> type, <span class="pl-c1">int8_t</span> *empty_pos)
{
    *empty_pos = -<span class="pl-c1">1</span>;
    <span class="pl-k">for</span> (<span class="pl-k">unsigned</span> i = <span class="pl-c1">0</span>; i < CREDMAN_MAX_CREDENTIALS; i++) {
        <span class="pl-c1">credman_credential_t</span> *c = &credentials[i];
        <span class="pl-k">if</span> ((c-><span class="pl-smi">tag</span> == tag) && (c-><span class="pl-smi">type</span> == type)) {
            <span class="pl-k">return</span> i;
        }
        <span class="pl-c"><span class="pl-c">/*</span> only check until empty position found <span class="pl-c">*/</span></span>
        <span class="pl-k">if</span> ((empty_pos) && (*empty_pos < <span class="pl-c1">0</span>) &&
            (c-><span class="pl-smi">tag</span> == CREDMAN_TAG_EMPTY) && (c-><span class="pl-smi">type</span> == CREDMAN_TYPE_EMPTY)) {
            *empty_pos = i;
        }
    }
    <span class="pl-k">return</span> -<span class="pl-c1">1</span>;
}

...

<span class="pl-c"><span class="pl-c">/*</span> check if credential already exist and find empty position simultaneously <span class="pl-c">*/</span></span>
<span class="pl-c1">int8_t</span> empty_pos;
    pos = _find_credential_pos(credential->tag, credential->type, &empty_pos);
    <span class="pl-k">if</span> (pos >= <span class="pl-c1">0</span>) {
        <span class="pl-c1">DEBUG</span>(<span class="pl-s"><span class="pl-pds">"</span>credman: credential with tag <span class="pl-c1">%d</span> and type <span class="pl-c1">%d</span> already exist<span class="pl-cce">\n</span><span class="pl-pds">"</span></span>,
              credential-><span class="pl-smi">tag</span>, credential-><span class="pl-smi">type</span>);
        ret = CREDMAN_EXIST;
        <span class="pl-k">goto</span> end;
    }
    <span class="pl-k">if</span> (empty_pos < <span class="pl-c1">0</span>) {
        <span class="pl-c1">DEBUG</span>(<span class="pl-s"><span class="pl-pds">"</span>credman: no space for new credential<span class="pl-cce">\n</span><span class="pl-pds">"</span></span>);
        ret = CREDMAN_NO_SPACE;
        <span class="pl-k">goto</span> end;
    }</pre></div>
</blockquote>
<p>How about the following simplification (also note, that I was able to do all without <code>goto</code>; I kept the label for the error handling above though):</p>
<div class="highlight highlight-source-c"><pre><span class="pl-c"><span class="pl-c">/*</span> Find position of credential with given tag and type.</span>
<span class="pl-c"> * To avoid going through the list twice, this function also returns the</span>
<span class="pl-c"> * first empty credential pool entry found.</span>
<span class="pl-c"> <span class="pl-c">*/</span></span>
<span class="pl-k">static</span> <span class="pl-k">int</span> <span class="pl-en">_find_credential_pos</span>(<span class="pl-c1">credman_tag_t</span> tag, <span class="pl-c1">credman_type_t</span> type,
<span class="pl-c1">credman_credential_t</span> **empty)
{
    <span class="pl-k">for</span> (<span class="pl-k">unsigned</span> i = <span class="pl-c1">0</span>; i < CREDMAN_MAX_CREDENTIALS; i++) {
        <span class="pl-c1">credman_credential_t</span> *c = &credentials[i];
        <span class="pl-k">if</span> ((c-><span class="pl-smi">tag</span> == tag) && (c-><span class="pl-smi">type</span> == type)) {
            <span class="pl-k">return</span> i;
        }
        <span class="pl-c"><span class="pl-c">/*</span> only check until empty position found <span class="pl-c">*/</span></span>
        <span class="pl-k">if</span> ((empty) && (*empty != <span class="pl-c1">NULL</span>) &&
            (c-><span class="pl-smi">tag</span> == CREDMAN_TAG_EMPTY) && (c-><span class="pl-smi">type</span> == CREDMAN_TYPE_EMPTY)) {
            *empty = c;
        }
    }
    <span class="pl-k">return</span> -<span class="pl-c1">1</span>;
}

...

<span class="pl-k">int</span> <span class="pl-en">credman_add</span>(<span class="pl-k">const</span> <span class="pl-c1">credman_credential_t</span> *credential)
{
    <span class="pl-c1">credman_credential_t</span> *entry = <span class="pl-c1">NULL</span>;
    <span class="pl-c1">assert</span>(credential);
...
    pos = <span class="pl-c1">_find_credential_pos</span>(credential-><span class="pl-smi">tag</span>, credential-><span class="pl-smi">type</span>, &entry);
    <span class="pl-k">if</span> (pos >= <span class="pl-c1">0</span>) {
        <span class="pl-c1">DEBUG</span>(<span class="pl-s"><span class="pl-pds">"</span>credman: credential with tag <span class="pl-c1">%d</span> and type <span class="pl-c1">%d</span> already exist<span class="pl-cce">\n</span><span class="pl-pds">"</span></span>,
              credential-><span class="pl-smi">tag</span>, credential-><span class="pl-smi">type</span>);
        ret = CREDMAN_EXIST;
    }
    <span class="pl-k">else</span> <span class="pl-k">if</span> (entry == <span class="pl-c1">NULL</span>) {
        <span class="pl-c1">DEBUG</span>(<span class="pl-s"><span class="pl-pds">"</span>credman: no space for new credential<span class="pl-cce">\n</span><span class="pl-pds">"</span></span>);
        ret = CREDMAN_NO_SPACE;
    }
    <span class="pl-k">else</span> {
        *entry = *credential;
        used++;
        ret = CREDMAN_OK;
    }
end:
    <span class="pl-c1">mutex_unlock</span>(&_mutex);
    <span class="pl-k">return</span> ret;
}</pre></div>

<p style="font-size:small;-webkit-text-size-adjust:none;color:#666;">—<br />You are receiving this because you are subscribed to this thread.<br />Reply to this email directly, <a href="https://github.com/RIOT-OS/RIOT/pull/11564?email_source=notifications&email_token=ABE7WYG37OAMUY2XON4EYTDQAFV2FA5CNFSM4HOVXRX2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB66TA7I#discussion_r305236025">view it on GitHub</a>, or <a href="https://github.com/notifications/unsubscribe-auth/ABE7WYBDCFFR4TLAM36LXLLQAFV2FANCNFSM4HOVXRXQ">mute the thread</a>.<img src="https://github.com/notifications/beacon/ABE7WYD67AYYCRI4A2F5ID3QAFV2FA5CNFSM4HOVXRX2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB66TA7I.gif" height="1" width="1" alt="" /></p>
<script type="application/ld+json">[
{
"@context": "http://schema.org",
"@type": "EmailMessage",
"potentialAction": {
"@type": "ViewAction",
"target": "https://github.com/RIOT-OS/RIOT/pull/11564?email_source=notifications\u0026email_token=ABE7WYG37OAMUY2XON4EYTDQAFV2FA5CNFSM4HOVXRX2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB66TA7I#discussion_r305236025",
"url": "https://github.com/RIOT-OS/RIOT/pull/11564?email_source=notifications\u0026email_token=ABE7WYG37OAMUY2XON4EYTDQAFV2FA5CNFSM4HOVXRX2YY3PNVWWK3TUL52HS4DFWFIHK3DMKJSXC5LFON2FEZLWNFSXPKTDN5WW2ZLOORPWSZGOB66TA7I#discussion_r305236025",
"name": "View Pull Request"
},
"description": "View this Pull Request on GitHub",
"publisher": {
"@type": "Organization",
"name": "GitHub",
"url": "https://github.com"
}
}
]</script>