From 4eee60343323499d087709614d47e24f66437697 Mon Sep 17 00:00:00 2001 From: Marco Costalba Date: Sat, 22 Mar 2014 11:15:28 +0100 Subject: [PATCH] Fix a subtle bug in UCI options printing We want all the UCI options are printed in the order in which are assigned, so we use an index that, depending on Options.size(), increases after each option is added to the map. The problem is that, for instance, in the first assignment: o["Write Debug Log"] = Option(false, on_logger); Options.size() can value 0 or 1 according if the l-value (that increments the size) has been evaluated after or before the r-value (that uses the size value). The culprit is that assignment operator in C++ is not a sequence point: http://en.wikipedia.org/wiki/Sequence_point (Note: to be nitpick here we actually use std::map::operator=() that being a function can evaluate its arguments in any order) So there is no guarantee on what term is evaluated first and behavior is undefined by standard in this case. The net result is that in case r-value is evaluated after l-value the last idx is not size() - 1, but size() and in the printing loop we miss the last option! Bug was there since ages but only recently has been exposed by the removal of UCI_Analyze option so that the last one becomes UCI_Chess960 and when it is missing engine cannot play anymore Chess960. The fix is trivial (although a bit hacky): just increase the last loop index. Reported by Eric Mullins that found it on an ARM and MIPS platforms with gcc 4.7 No functional change. --- src/ucioption.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ucioption.cpp b/src/ucioption.cpp index eefacb22..062b26d7 100644 --- a/src/ucioption.cpp +++ b/src/ucioption.cpp @@ -92,7 +92,7 @@ void init(OptionsMap& o) { std::ostream& operator<<(std::ostream& os, const OptionsMap& om) { - for (size_t idx = 0; idx < om.size(); ++idx) + for (size_t idx = 0; idx < om.size() + 1; ++idx) // idx could start from 1 for (OptionsMap::const_iterator it = om.begin(); it != om.end(); ++it) if (it->second.idx == idx) { -- 2.39.2