Bugzilla – Bug 1132
useless for loops in block-ack-test-suite.cc
Last modified: 2015-09-18 05:22:04 UTC
this is a (probable) error spotted when running check-style.py - for (i = m_buffer.begin (); i != m_buffer.end () && QosUtilsMapSeqControlToUniqueInteger ((*i), endSeq) < mappedSeq; i++); - { - m_buffer.insert (i, receivedSeq); - } + for (i = m_buffer.begin (); i != m_buffer.end () && QosUtilsMapSeqControlToUniqueInteger ((*i), endSeq) < mappedSeq; i++) + { + ; + } + { + m_buffer.insert (i, receivedSeq); + }
Created attachment 1106 [details] naive tentative patch that makes the test fail (note: as of 7142:89a701fec3a1, check-style.py has been run on wifi, so in the previously cited code from src/wifi/test/block-ack-test-suite.cc the useless "for" loops are now more evident) the attached patch is based on the assumption that the original error was just a typo, i.e., the "for" loops were useless just because of an additional semi-colon added by mistake. So this patch does the equivalent of eliminating the original semi-colon and making the "for" look iterate over the subsequent code between braces "{}". Unfortunately, the resulting test program fails. More investigation is needed.
Mirko, you are the original author of this code, could you please take a look?
Hi Nicola...Yes,of course those semi-colons are errors...however now i note in ns-3-dev your changes: for (i = m_buffer.begin (); i != m_buffer.end () && QosUtilsMapSeqControlToUniqueInteger ((*i), endSeq) < mappedSeq; i++) { ; } { m_buffer.insert (i, receivedSeq); } an the same for the other loop...this is obviously wrong...Is this only a way to pass tests? However i'll investigate ASAP.
(In reply to comment #3) > Hi Nicola...Yes,of course those semi-colons are errors...however now i note in > ns-3-dev your changes: > > for (i = m_buffer.begin (); i != m_buffer.end () && > QosUtilsMapSeqControlToUniqueInteger ((*i), endSeq) < mappedSeq; i++) > { > ; > } > > { > m_buffer.insert (i, receivedSeq); > } > > an the same for the other loop...this is obviously wrong...Is this only a way > to pass tests? However i'll investigate ASAP. The no-op block { ; } was added automatically by check-style.py to replace the single ";" at the end of the "for" line. I didn't do any manual change. Functionally, they are equivalent. The patch that I attached above aims at removing the no-op block (which is equivalent to removing the ";" on the "for" line in the original code). But with this patch, the test doesn't pass. Hence I think there is an error in the test.
(In reply to comment #4) > (In reply to comment #3) > > Hi Nicola...Yes,of course those semi-colons are errors...however now i note in > > ns-3-dev your changes: > > > > for (i = m_buffer.begin (); i != m_buffer.end () && > > QosUtilsMapSeqControlToUniqueInteger ((*i), endSeq) < mappedSeq; i++) > > { > > ; > > } > > > > { > > m_buffer.insert (i, receivedSeq); > > } > > > > an the same for the other loop...this is obviously wrong...Is this only a way > > to pass tests? However i'll investigate ASAP. > > The no-op block > { > ; > } > was added automatically by check-style.py to replace the single ";" at the end > of the "for" line. I didn't do any manual change. Functionally, they are > equivalent. > > The patch that I attached above aims at removing the no-op block (which is > equivalent to removing the ";" on the "for" line in the original code). But > with this patch, the test doesn't pass. Hence I think there is an error in the > test. Of course...you are right! I observed it very quickly...sorry. However, again, i'll investigate asap. Thank you Nicola!
Mirko, what are the updates on this bug?
Mirko is having a look
fixed in changeset 11671:4ef2a0434d99