pimd icon indicating copy to clipboard operation
pimd copied to clipboard

Misleading comment in add_rp_grp_entry(), suggests missing required processing

Open jp-t opened this issue 7 years ago • 2 comments

Since the RP hold time was fixed in RP #106, this comment in rp.c around line 380:

        /* TODO: We shoudn't have old existing entry, because with the
         * current implementation all of them will be deleted
         * (different fragment_tag). Debug and check and eventually
         * delete.
         */

is not true anymore. A debug log message in that place will appear very often.

  • Contrary to the suggestion in the comment, the code here should not be removed.
  • Since the current behavior is new, the code is at best untested and might be incomplete or buggy. However a simple test with two routers linking a MC sender and a receiver, do work properly.

Need to investigate and correct/remove the comment.

jp-t avatar Jul 10 '17 14:07 jp-t

Reading the whole function, it seems that the RP priority is not updated when an old rp_grp_entry is found, and if priority changed, the RP<=>group remapping does not take place. This might become an issue in corner cases where RP priorities change while running.

jp-t avatar Jul 10 '17 15:07 jp-t

Thank you for following up and reporting this!

troglobit avatar Jul 10 '17 15:07 troglobit