Fix a subtle bug in UCI options printing
authorMarco Costalba <mcostalba@gmail.com>
Sat, 22 Mar 2014 10:15:28 +0000 (11:15 +0100)
committerMarco Costalba <mcostalba@gmail.com>
Sat, 22 Mar 2014 10:30:06 +0000 (11:30 +0100)
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

index eefacb22bc447293ae168923461614c16a68b7aa..062b26d70649a9528645de856ba35452e3c30c5f 100644 (file)
@@ -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)
           {