Page 6 of 9

Re: UCI2WB 4.0

Posted: Fri Jun 14, 2019 9:49 am
by Ferdy
Thanks for this new update, downloaded the winboard-aa pack, what is the best way to remove the engines in the list? I plan to add new engines but have to remove first all existing engines read by winboard.

Image

Re: UCI2WB 4.0

Posted: Fri Jun 14, 2019 1:03 pm
by Ferdy
hgm wrote: Wed Jun 12, 2019 3:30 pm I now also updated the WinBoard executable in the AA package.
Run some test using the tournament manager. It is not stable so far. But the games are saved.

Image

Re: UCI2WB 4.0

Posted: Fri Jun 14, 2019 2:25 pm
by hgm
Thank you for testing.

Best way to remove engines is by Engine->Edit Engine List, and just remove all the lines with the engines you want to delete.

Strange that WinBoard crashes. I did not really change anything in it that should be executed when running games and tournaments. (Just redesigned some dialogs, and how mouse clicks are handled during Edit Position.) I wonder if UCI2WB could somehow crash WinBoard. Perhaps it floods WinBoard with unintended output which overruns some buffer.

The screenshot shows the crashes happen at the end of a game: both the crashed windows have a result message in the display. Were any of the involved engines actually running under UCI2WB? (The title bar doesn't mention it, while it does so in the Cheng-Deuterion game.)

Re: UCI2WB 4.0

Posted: Fri Jun 14, 2019 3:30 pm
by Ras
hgm wrote: Fri Jun 14, 2019 2:25 pmPerhaps it floods WinBoard with unintended output which overruns some buffer.
A function like this (from UCI2WB) looks like a buffer overflow waiting to happen:

Code: Select all

int
ReadLine (FILE *f, char *line)
{
    int x, i = 0;
    while((x = fgetc(f)) != EOF && (line[i] = x) != '\n') i++; line[++i] = 0;
    return (x != EOF);
}
And CppCheck 1.87 warns that sscanf() without input field limit is also dangerous.

Re: UCI2WB 4.0

Posted: Fri Jun 14, 2019 3:39 pm
by Ferdy
hgm wrote: Fri Jun 14, 2019 2:25 pm Thank you for testing.

Best way to remove engines is by Engine->Edit Engine List, and just remove all the lines with the engines you want to delete.

Strange that WinBoard crashes. I did not really change anything in it that should be executed when running games and tournaments. (Just redesigned some dialogs, and how mouse clicks are handled during Edit Position.) I wonder if UCI2WB could somehow crash WinBoard. Perhaps it floods WinBoard with unintended output which overruns some buffer.

The screenshot shows the crashes happen at the end of a game: both the crashed windows have a result message in the display. Were any of the involved engines actually running under UCI2WB? (The title bar doesn't mention it, while it does so in the Cheng-Deuterion game.)
After deleting the engines in the list, now I could not enter the GUI.
Image

In the tour with crashed winboard, engines with -fn
-fn "Deuterium v2019.1.36.50" -fUCI
-fn "Dirty CUCUMBER"
-fn "Bobcat 8.0" -fUCI
-fn "Ethereal 9.00" -fUCI

Re: UCI2WB 4.0

Posted: Fri Jun 14, 2019 4:59 pm
by hgm
Doesn't ticking 'Just view or edit game files' work?

In principle you can also type an engine line in the comboboxes, but as that is not put in the list you probably wouldn't want to do that.

About the crashes: do they only happen when you run multiple instances of WB on the same tourney? I tried a small round robin with 2 Stockfishes and two Fairy-Maxes, and nothing irregular happened.

Re: UCI2WB 4.0

Posted: Sat Jun 15, 2019 6:53 am
by Ferdy
hgm wrote: Fri Jun 14, 2019 4:59 pm Doesn't ticking 'Just view or edit game files' work?
Right I am able to open it. Add one engine on the list, save settings, close winboard, open winboard again, and the added engine is not visible. Open via via just view or edit ..., add another engine, save settings, close winboard, open winboard again and got this.

Image

After selecting any of the engines and press OK nothing happens.

Re: UCI2WB 4.0

Posted: Sat Jun 15, 2019 7:13 am
by hgm
It seems that the first line in the engine list has become an empty one in the process of editing it. Edit the list again and delete that line.

WinBoard has always insisted that you specify both engines in the Startup Dialog. Even though you might not plan doing engine-engine games at all, which is the only case where the second engine is used. (This unlike XBoard, which has no Startup Dialog, and has always used a pre-programmed default engine when no -fcp or -scp argument was given. At some point I changed this to using the -fcp value as default for -scp, which avoids picking a second engine by default that does not support the same variants as the first.)

So if the default choice is empty, you must also pick something for the second engine.

This could be considered undesirable behavior. I could imagine there are situations where someone doesn't want a preconfigured default but force a conscious choice. But then it is annoying to also require one for the second engine, which you might never want to use. So perhaps I should also implement the XBoard behavior here: when the choice for the second engine is an empty line it could automatically be replaced by choice for the first engine.

Re: UCI2WB 4.0

Posted: Sat Jun 15, 2019 7:43 am
by Ferdy
hgm wrote: Sat Jun 15, 2019 7:13 am It seems that the first line in the engine list has become an empty one in the process of editing it. Edit the list again and delete that line.
Tried it and got this, "I" was missing.

Image

Removed the empty line and add "I", save, OK. But then when I press edit engine list again, I got same image above, so my changes were not saved.

Re: UCI2WB 4.0

Posted: Sat Jun 15, 2019 7:48 am
by hgm
Ras wrote: Fri Jun 14, 2019 3:30 pmA function like this (from UCI2WB) looks like a buffer overflow waiting to happen:
Umm yes, that is a bit sloppy. Especially as the buffer size is not extremely generous; for reading from the engine it is 1024 bytes, and although I have never seen an engine spit out a PV of more than 200 moves, UCI specs don't really forbid it. I guess I will replace the line[ i] by line[ i-=(i>1022)] just to be save.

I don't think this is related to the crashing of WinBoard, though.
And CppCheck 1.87 warns that sscanf() without input field limit is also dangerous.
For reading strings that is true, but the engine is considered a "trusted agent", which would not do non-compliant or unreasonable things (such as option name longer than 80 characters, which would create tons of problems further down the line in the GUI anyway). UCI was not intended to be a compliancy checker for UCI, and non-compliancy in principle causes undefined behavior.