[riot-notifications] [RIOT-OS/RIOT] net/nanocoap: fix confirmable retry countdown (#10671)

Ken Bannister notifications at github.com
Mon Jan 7 20:05:32 CET 2019


kb2ma commented on this pull request.



> @@ -46,19 +46,14 @@ ssize_t nanocoap_request(coap_pkt_t *pkt, sock_udp_ep_t *local, sock_udp_ep_t *r
 
     /* TODO: timeout random between between ACK_TIMEOUT and (ACK_TIMEOUT *
      * ACK_RANDOM_FACTOR) */
-    uint32_t timeout = COAP_ACK_TIMEOUT * (1000000U);
-    int tries = 0;
-    while (tries++ < COAP_MAX_RETRANSMIT) {
-        if (!tries) {
-            DEBUG("nanocoap: maximum retries reached.\n");
-            res = -ETIMEDOUT;
-            goto out;
-        }
+    uint32_t timeout = COAP_ACK_TIMEOUT * (1 * US_PER_SEC);
+    unsigned tries_left = COAP_MAX_RETRANSMIT + 1;  /* add 1 for initial transmit */
+    while (tries_left) {

In my experience, use of the decrement operator on an unsigned value is error prone. In this case, how do I display the "maximum retries reached" debug message when max retransmit is zero? I tested, and it doesn't work. The decrement operator rolled it back over.

I'm OK with converting tries_left to a signed value, but then I would want to use an explicit inequality in the do test "tries_left-- > 0". If I don't the logic would look strange against the test for debug below.

```
    } while (tries_left-- > 0);

    if (tries_left <= 0) {
        DEBUG("nanocoap: maximum retries reached\n");
    }
```

I still like this solution better than my original because I think the overall flow is easier to follow. Let me know what you think.

-- 
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/10671#discussion_r245763547
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.riot-os.org/pipermail/notifications/attachments/20190107/be257cd2/attachment.html>


More information about the notifications mailing list