[riot-notifications] [RIOT-OS/RIOT] sys: add credman (D)TLS credential management module (#11564)

Martine Lenders notifications at github.com
Fri Jul 19 09:41:22 CEST 2019


miri64 commented on this pull request.



> +        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;
+    }

I can't find your comment on this in Github, but I found the following in my mails

> This is what I came up with based on suggestion from Cenk:
> ```C
> /* Find position of credential with given tag and type.
>  * To avoid going through the list twice, this function also returns the
>  * first empty position found.
>  */
> static int _find_credential_pos(credman_tag_t tag, credman_type_t type, int8_t *empty_pos)
> {
>     *empty_pos = -1;
>     for (unsigned i = 0; i < CREDMAN_MAX_CREDENTIALS; i++) {
>         credman_credential_t *c = &credentials[i];
>         if ((c->tag == tag) && (c->type == type)) {
>             return i;
>         }
>         /* only check until empty position found */
>         if ((empty_pos) && (*empty_pos < 0) &&
>             (c->tag == CREDMAN_TAG_EMPTY) && (c->type == CREDMAN_TYPE_EMPTY)) {
>             *empty_pos = i;
>         }
>     }
>     return -1;
> }
> 
> ...
> 
> /* check if credential already exist and find empty position simultaneously */
> int8_t empty_pos;
>     pos = _find_credential_pos(credential->tag, credential->type, &empty_pos);
>     if (pos >= 0) {
>         DEBUG("credman: credential with tag %d and type %d already exist\n",
>               credential->tag, credential->type);
>         ret = CREDMAN_EXIST;
>         goto end;
>     }
>     if (empty_pos < 0) {
>         DEBUG("credman: no space for new credential\n");
>         ret = CREDMAN_NO_SPACE;
>         goto end;
>     }
> ```

How about the following simplification (also note, that I was able to do all without `goto`; I kept the label for the error handling above though):

```C
/* Find position of credential with given tag and type.
 * To avoid going through the list twice, this function also returns the
 * first empty credential pool entry found.
 */
static int _find_credential_pos(credman_tag_t tag, credman_type_t type,
credman_credential_t **empty)
{
    for (unsigned i = 0; i < CREDMAN_MAX_CREDENTIALS; i++) {
        credman_credential_t *c = &credentials[i];
        if ((c->tag == tag) && (c->type == type)) {
            return i;
        }
        /* only check until empty position found */
        if ((empty) && (*empty != NULL) &&
            (c->tag == CREDMAN_TAG_EMPTY) && (c->type == CREDMAN_TYPE_EMPTY)) {
            *empty = c;
        }
    }
    return -1;
}

...

int credman_add(const credman_credential_t *credential)
{
    credman_credential_t *entry = NULL;
    assert(credential);
...
    pos = _find_credential_pos(credential->tag, credential->type, &entry);
    if (pos >= 0) {
        DEBUG("credman: credential with tag %d and type %d already exist\n",
              credential->tag, credential->type);
        ret = CREDMAN_EXIST;
    }
    else if (entry == NULL) {
        DEBUG("credman: no space for new credential\n");
        ret = CREDMAN_NO_SPACE;
    }
    else {
        *entry = *credential;
        used++;
        ret = CREDMAN_OK;
    }
end:
    mutex_unlock(&_mutex);
    return ret;
}

-- 
You are receiving this because you are subscribed to this thread.
Reply to this email directly or view it on GitHub:
https://github.com/RIOT-OS/RIOT/pull/11564#discussion_r305236025
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190719/dbd2caab/attachment-0001.htm>


More information about the notifications mailing list