Bug 1681 - m_lastNavStart is assigned twice continuously
m_lastNavStart is assigned twice continuously
Product: ns-3
Classification: Unclassified
Component: wifi
All All
: P5 major
Assigned To: sebastien.deronne
Depends on:
  Show dependency treegraph
Reported: 2013-05-12 22:21 EDT by Junling Bu
Modified: 2015-09-11 15:38 EDT (History)
2 users (show)

See Also:


Note You need to log in before you can comment on or make changes to this bug.
Description Junling Bu 2013-05-12 22:21:10 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;".
Comment 1 Nicola Baldo 2013-05-25 17:36:58 EDT
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.
Comment 2 Junling Bu 2013-05-27 08:13:16 EDT
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 :)
Comment 3 Nicola Baldo 2015-04-07 10:17:27 EDT
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.
Comment 4 sebastien.deronne 2015-09-11 15:38:27 EDT
This was fixed in changeset 11543:ec5dd7666b77