ccnl-riot: initialize fields for default options #209
Conversation
|
This is slightly related to the PR, but I did a quick check and the actual default value of the interest lifetime in NDN is 4 seconds and not 10 seconds. What is the rational? Is it different in other ICN flavors which provide a interest lifetime? |
| @@ -547,6 +547,9 @@ ccnl_send_interest(struct ccnl_prefix_s *prefix, unsigned char *buf, int buf_len | |||
| int ret = -1; | |||
| int len = 0; | |||
| ccnl_interest_opts_u default_opts; | |||
| default_opts.ndntlv.nonce = 0; | |||
| default_opts.ndntlv.mustbefresh = false; | |||
| default_opts.ndntlv.interestlifetime = CCNL_INTEREST_TIMEOUT; | |||
There was a problem hiding this comment.
Have you checked if the value is set later on in milliseconds? The define points to a constant which is in seconds, but the specification expects the value to be in milliseconds.
There was a problem hiding this comment.
No, I haven't. I just checked that this value is asigned in src/ccnl-pkt/src/ccnl-pkt-ndntlv.c in ccnl_ndntlv_bytes2pkt() as a default value.
There was a problem hiding this comment.
I have to check it (it's quite confusing). In ccnl_interest_new in ccnl_relay.c the value is divided by 1000 (so it is seconds).
There was a problem hiding this comment.
So this pull request is correct and can be applied?
I am not sure if you want to have nonce = 0 as default.
There was a problem hiding this comment.
Ok, in line 570 a nonce will be generated if it was set to zero.
| @@ -547,6 +547,9 @@ ccnl_send_interest(struct ccnl_prefix_s *prefix, unsigned char *buf, int buf_len | |||
| int ret = -1; | |||
| int len = 0; | |||
| ccnl_interest_opts_u default_opts; | |||
| default_opts.ndntlv.nonce = 0; | |||
| default_opts.ndntlv.mustbefresh = false; | |||
| default_opts.ndntlv.interestlifetime = CCNL_INTEREST_TIMEOUT; | |||
There was a problem hiding this comment.
Ok, in line 570 a nonce will be generated if it was set to zero.
|
I don't think that this was actually a very good idea on multiple levels. Again, the interest lifetime is expected to be in milliseconds and not seconds ( The Could we revert the merge? This is clearly not what we want (sending out interests with a second granularity). |
|
But as far as I can see, it does not changes the behavior we already have? |
No description provided.