Bugzilla – Bug 1681
m_lastNavStart is assigned twice continuously
Last modified: 2015-09-11 15:38:27 EDT
in src/wifi/model/mac-low.cc 1059 m_lastNavStart = Simulator::Now (); 1060 m_lastNavStart = duration; Although I haven't completely understood codes of mac-low.cc now, I think we should change "m_lastNavStart = duration;" of line 1060 to "m_lastNavDuration = duration;".
Looking at the mercurial history of that line, I found out that a patch identical to yours was proposed as a fix to bug 584, and committed as changeset d21a2eafb84d. However shortly after Mathieu backed out this change in changeset d7e974543905. The fact that the patch was backed out suggests that it was not correct. I understand that the change might seem reasonable by looking at other parts of the same file (e.g., lines 1073-1074), however I think we should not accept this patch again without some more detailed analysis and some careful testing. I've seen many other times some patches to the wifi mac-low and dcf code that seemed straightforward at first sight but turned out to give problems (see bug 555 for some examples) so I really think that we have to proceed with caution.
Thanks for your comments. 1. I read the bugs you indicated, I think I need more time to understand the details of mac low layer. 2. I understand your cautiousness that needs more consideration for developer and maintainer. But I am still confused with the patch that was backed out. Even it was not correct , it should be wrong in other part, and need us to fix. It's just my personal thought. 3. If this is not the bug, I suggest to add some comments there , because if someone reads source codes of mac-low.cc carefully, he will take this as a bug :)
the issue still present in MacLow::DoNavResetNow (Time duration) even if there is no bug (in the sense that the current implementation works correctly) we should at least add a comment in the code. Or perhaps we should remove the first assignment, which I guess is useless since the variable is of the simple type Time which I don't think has any side-effect invoked upon assignment.
This was fixed in changeset 11543:ec5dd7666b77