From 923c9a32ef9ef77553c7ef1bbdf55e5b090d23bd Mon Sep 17 00:00:00 2001
From: Benjamin Valentin <benjamin.valentin@ml-pa.com>
Date: Wed, 7 Jun 2023 13:02:13 +0200
Subject: [PATCH] nanocoap: clean up coap_iterate_option(), make it public

---
 sys/include/net/nanocoap.h                    | 15 +++++
 sys/net/application_layer/nanocoap/nanocoap.c | 64 +++++++++++--------
 2 files changed, 53 insertions(+), 26 deletions(-)

diff --git a/sys/include/net/nanocoap.h b/sys/include/net/nanocoap.h
index 1fc958c366..7ee77b88d7 100644
--- a/sys/include/net/nanocoap.h
+++ b/sys/include/net/nanocoap.h
@@ -656,6 +656,21 @@ static inline void coap_hdr_set_type(coap_hdr_t *hdr, unsigned type)
  */
 uint8_t *coap_find_option(coap_pkt_t *pkt, unsigned opt_num);
 
+/**
+ * @brief   Get pointer to an option field, can be called in a loop
+ *          if there are multiple options with the same number
+ *
+ * @param[in]   pkt     packet to work on
+ * @param[in]   opt_num the option number to search for
+ * @param[out]  opt_pos opaque, must be set to `NULL` on first call
+ * @param[out]  opt_len size of the current option data
+ *
+ * @returns     pointer to the option data
+ *              NULL if option number was not found
+ */
+uint8_t *coap_iterate_option(coap_pkt_t *pkt, unsigned opt_num,
+                             uint8_t **opt_pos, int *opt_len);
+
 /**
  * @brief   Get content type from packet
  *
diff --git a/sys/net/application_layer/nanocoap/nanocoap.c b/sys/net/application_layer/nanocoap/nanocoap.c
index 7a83124b59..30ed3d32ed 100644
--- a/sys/net/application_layer/nanocoap/nanocoap.c
+++ b/sys/net/application_layer/nanocoap/nanocoap.c
@@ -281,19 +281,27 @@ int coap_opt_get_uint(coap_pkt_t *pkt, uint16_t opt_num, uint32_t *target)
     return -ENOENT;
 }
 
-uint8_t *coap_iterate_option(coap_pkt_t *pkt, uint8_t **optpos,
-                             int *opt_len, int first)
+uint8_t *coap_iterate_option(coap_pkt_t *pkt, unsigned optnum,
+                             uint8_t **opt_pos, int *opt_len)
 {
     uint8_t *data_start;
 
+    bool first = false;
+    if (*opt_pos == NULL) {
+        *opt_pos = coap_find_option(pkt, optnum);
+        first = true;
+        if (*opt_pos == NULL) {
+            return NULL;
+        }
+    }
+
     uint16_t delta = 0;
-    data_start = _parse_option(pkt, *optpos, &delta, opt_len);
+    data_start = _parse_option(pkt, *opt_pos, &delta, opt_len);
     if (data_start && (first || !delta)) {
-        *optpos = data_start + *opt_len;
+        *opt_pos = data_start + *opt_len;
         return data_start;
     }
     else {
-        *optpos = NULL;
         return NULL;
     }
 }
@@ -359,31 +367,35 @@ ssize_t coap_opt_get_string(coap_pkt_t *pkt, uint16_t optnum,
 {
     assert(pkt && target && (max_len > 1));
 
-    uint8_t *opt_pos = coap_find_option(pkt, optnum);
-    if (!opt_pos) {
+    uint8_t *opt_pos = NULL;
+    int opt_len;
+    unsigned left = max_len;
+
+    while (1) {
+        uint8_t *part_start = coap_iterate_option(pkt, optnum, &opt_pos, &opt_len);
+
+        if (part_start == NULL) {
+            /* if option was not found still return separator */
+            if (opt_pos == NULL) {
+                *target++ = (uint8_t)separator;
+                --left;
+            }
+            break;
+        }
+
+        /* separator and terminating \0 have to fit */
+        if (left < (unsigned)(opt_len + 2)) {
+            return -ENOSPC;
+        }
+
         *target++ = (uint8_t)separator;
-        *target = '\0';
-        return 2;
+        memcpy(target, part_start, opt_len);
+        target += opt_len;
+        left -= opt_len + 1;
     }
 
-    unsigned left = max_len - 1;
-    uint8_t *part_start = NULL;
-    do {
-        int opt_len;
-        part_start = coap_iterate_option(pkt, &opt_pos, &opt_len,
-                                         (part_start == NULL));
-        if (part_start) {
-            if (left < (unsigned)(opt_len + 1)) {
-                return -ENOSPC;
-            }
-            *target++ = (uint8_t)separator;
-            memcpy(target, part_start, opt_len);
-            target += opt_len;
-            left -= (opt_len + 1);
-        }
-    } while (opt_pos);
-
     *target = '\0';
+    left--;
 
     return (int)(max_len - left);
 }