[Freeciv-Dev] [bug #20744] Recursive wipe_unit() saving drowning units from calling wipe_unit()
Update of bug #20744 (project freeciv): Status: Ready For Test = Fixed Assigned to:None = cazfi Open/Closed:Open = Closed ___ Reply to this item at: http://gna.org/bugs/?20744 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20744] Recursive wipe_unit() saving drowning units from calling wipe_unit()
Follow-up Comment #3, bug #20744 (project freeciv): As we're heading to beta2, I'll commit this with extra argument intact. That's what's been extensively tested (like in: several autogames (on multicore system) constantly running almost from the day trunk patch was first introduced). We may cleanup it away from S2_4 version after the beta, if someone bothers. ___ Reply to this item at: http://gna.org/bugs/?20744 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20744] Recursive wipe_unit() saving drowning units from calling wipe_unit()
Update of bug #20744 (project freeciv): Planned Release: 2.4.0, 2.5.0, 2.6.0 = 2.4.0-beta2, 2.5.0, 2.6.0 ___ Follow-up Comment #4: Correction: Had to make new version anyway due to other changes to codebase, so removed S2_4 helpless while at it. (file #17791, file #17792) ___ Additional Item Attachment: File name: WipeUnitLists-2.patch Size:8 KB File name: WipeUnitLists-S2_4-2.patch Size:8 KB ___ Reply to this item at: http://gna.org/bugs/?20744 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20744] Recursive wipe_unit() saving drowning units from calling wipe_unit()
Follow-up Comment #5, bug #20744 (project freeciv):
I think it's safe either way: I just tend to avoid having unused arguments or
functions if possible, to ease later reading of the code (but given the volume
of testing to which this was subjected, there is a strong argument for leaving
it intact).
Looking at the most recent revisions of the patch, I wonder if
unit_list_iterate_safe(remaining, pcargo){} should be guarded with a check to
ensure unit_list_size(remaining) 0 (this would apply to either S2_4 or trunk
patches).
___
Reply to this item at:
http://gna.org/bugs/?20744
___
Message sent via/by Gna!
http://gna.org/
___
Freeciv-dev mailing list
Freeciv-dev@gna.org
https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20744] Recursive wipe_unit() saving drowning units from calling wipe_unit()
URL: http://gna.org/bugs/?20744 Summary: Recursive wipe_unit() saving drowning units from calling wipe_unit() Project: Freeciv Submitted by: cazfi Submitted on: Fri 19 Apr 2013 11:52:39 AM EEST Category: general Severity: 3 - Normal Priority: 5 - Normal Status: Ready For Test Assigned to: None Originator Email: Open/Closed: Open Release: Discussion Lock: Any Operating System: None Planned Release: 2.4.0, 2.5.0, 2.6.0 ___ Details: Haven't tried to setup test case, but when reading wipe_unit() refactoring from patch #3804 (use unit lists to store drowning units instead of just having count of them and then rechecking which units they are) it occurred to me that recursively called wipe_unit() might handle those units calling wipe_unit() has counted to its 'drowning' counter, meaning that the counter value is not correct (in practice: counter instructs to wipe units as drowning ones even after all have been saved) Using the lists the way refactoring part of patch #3804 does avoids such problems. ___ Reply to this item at: http://gna.org/bugs/?20744 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20744] Recursive wipe_unit() saving drowning units from calling wipe_unit()
Follow-up Comment #1, bug #20744 (project freeciv): - S2_4 version of the patch (file #17768) ___ Additional Item Attachment: File name: WipeUnitLists-S2_4.patch Size:8 KB ___ Reply to this item at: http://gna.org/bugs/?20744 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
[Freeciv-Dev] [bug #20744] Recursive wipe_unit() saving drowning units from calling wipe_unit()
Follow-up Comment #2, bug #20744 (project freeciv): try_to_save_unit() doesn't need to take the helpless argument for S2_4, as this only exists as a base for future extension, but leaving it there should cause no problem and it may be helpful to have the S2_4 and S2_5 functions be identical to reduce rework if an issue is discovered. ___ Reply to this item at: http://gna.org/bugs/?20744 ___ Message sent via/by Gna! http://gna.org/ ___ Freeciv-dev mailing list Freeciv-dev@gna.org https://mail.gna.org/listinfo/freeciv-dev
