I suppose it is subjective. This is a short simple loop, the 'loop til found' idiom for break is quite common, and it saves the (IMO awkward) placement of the ++i in the else clause only for the corner case where you're about to go up and realize that you just set success to true so you're going to exit the loop anyway, so to me this situation is tailor-made for break. Or in other words, if you don't use a break here, I can't imagine that you would ever use it anywhere, which may well be your preference.
Up to you... just thought I'd suggest it, since I personally would not shy away from break in this case :). Steve On Mon, Feb 15, 2016 at 9:59 AM Andreas Hansson <[email protected]> wrote: > It’s definitely an option. I’m not a massive fan of break/continue myself > (multiple exit points and all that). > > Andreas > > On 15/02/2016, 17:49, "gem5-dev on behalf of Steve Reinhardt" > <[email protected] on behalf of [email protected]> wrote: > > >Would it not be cleaner just to use a 'break' at the end of the if clause? > > > >Steve > > > > > >On Mon, Feb 15, 2016 at 12:58 AM Andreas Hansson <[email protected] > > > >wrote: > > > >> changeset dc0ed2d4da50 in /z/repo/gem5 > >> details: http://repo.gem5.org/gem5?cmd=changeset;node=dc0ed2d4da50 > >> description: > >> mem: Avoid using invalid iterator in cache lock list traversal > >> > >> Fix up issue highlighted by Valgrind and the clang Address > >> Sanitizer. > >> > >> diffstat: > >> > >> src/mem/cache/blk.hh | 5 +++-- > >> 1 files changed, 3 insertions(+), 2 deletions(-) > >> > >> diffs (22 lines): > >> > >> diff -r c45bfadcd51b -r dc0ed2d4da50 src/mem/cache/blk.hh > >> --- a/src/mem/cache/blk.hh Sun Feb 14 20:28:48 2016 -0500 > >> +++ b/src/mem/cache/blk.hh Mon Feb 15 03:40:04 2016 -0500 > >> @@ -347,15 +347,16 @@ > >> bool success = false; > >> > >> auto l = lockList.begin(); > >> - while (l != lockList.end() && !success) { > >> + while (!success && l != lockList.end()) { > >> if (l->matches(pkt->req)) { > >> // it's a store conditional, and as far as the > >> // memory system can tell, the requesting > >> // context's lock is still valid. > >> success = true; > >> lockList.erase(l); > >> + } else { > >> + ++l; > >> } > >> - ++l; > >> } > >> > >> req->setExtraData(success ? 1 : 0); > >> _______________________________________________ > >> gem5-dev mailing list > >> [email protected] > >> http://m5sim.org/mailman/listinfo/gem5-dev > >> > >_______________________________________________ > >gem5-dev mailing list > >[email protected] > >http://m5sim.org/mailman/listinfo/gem5-dev > > IMPORTANT NOTICE: The contents of this email and any attachments are > confidential and may also be privileged. If you are not the intended > recipient, please notify the sender immediately and do not disclose the > contents to any other person, use it for any purpose, or store or copy the > information in any medium. Thank you. > _______________________________________________ > gem5-dev mailing list > [email protected] > http://m5sim.org/mailman/listinfo/gem5-dev > _______________________________________________ gem5-dev mailing list [email protected] http://m5sim.org/mailman/listinfo/gem5-dev
