search

mate-amargo / pgn2fen

Extracts FEN of a specific move on a PGN game
GNU General Public License v3.0
8 stars 1 forks source link

Bug and Bugfix #1

Closed Thomas-- closed 4 years ago

Thomas-- commented 4 years ago

For some games the program produces incorrect output.

Input:

[Event "Grand Slam Chess Final"]
[Site "Sao Paulo BRA"]
[Date "2012.09.25"]
[Result "0-1"]
[White "Francisco Vallejo Pons"]
[Black "Magnus Carlsen"]
[PlyCount "82"]

1. e4 d6 2. d4 Nf6 3. Nc3 e5 4. dxe5 dxe5 5. Qxd8+ Kxd8 6. Nf3
Bd6 7. Bg5 Be6 8. O-O-O Nbd7 9. Nb5 Ke7 10. Nxd6 cxd6 11. Bb5
Rhd8 12. Nd2 h6 13. Bh4 g5 14. Bg3 a6 15. Bxd7 Rxd7 16. f3 Rc8
17. Kb1 Nh5 18. Nf1 f5 19. exf5 Bxf5 20. Ne3 Bg6 21. Rd2 Ke6
22. b3 b5 23. Kb2 d5 24. Re1 Nxg3 25. hxg3 h5 26. c3 d4
27. cxd4 Rxd4 28. Rxd4 exd4 29. Nc2+ Kd5 30. Nb4+ Kd6 31. Rc1
Rxc1 32. Kxc1 h4 33. gxh4 gxh4 34. Nxa6 Bd3 35. Nb4 Bf1
36. Kd2 Bxg2 37. Ke2 Bh3 38. a4 Bf5 39. axb5 d3+ 40. Ke3 h3
41. Nxd3 Bxd3 0-1

How to reproduce: For the above pgn the program produces an incorrect fen for black's 15th move: $ ./pgn2fen game.pgn 15 b 7/1p1rkp2/p2pbn1p/4p1p1/4P3/6B1/PPPN1PPP/2KR3R w - - 0 16 The correct output should start with r7/...

Bug: The program uses sprintf(&rook[1],"%d", RANKS - i); to convert the rank number into a character. However, sprintf appends a terminating null character. This null character is written into the memory location next to &rook[1] which (most likely) happens to be &board[0][0].

Bugfix: (It's been a while that I've written c/c++ code, but...) I think replacing the four sprintf lines in the source code with rook[1] = '0' + (RANKS - i); should fix the problem. I used $ sed -i 's/\([[:space:]]\)sprintf(&rook\[1\],"%d", RANKS - i);/\1rook\[1\] = '"'"'0'"'"' + (RANKS - i);/g' main.c to alter the source code before compiling (as described, with make) and then the program produces the correct output: r7/1p1rkp2/p2pbn1p/4p1p1/4P3/6B1/PPPN1PPP/2KR3R w - - 0 16

mate-amargo commented 4 years ago

Hello!

I can't seem to reproduce the problem on my machine. I used the example you provided but in my tests it produces the correct output. It's been more than 3 years since I coded that. I don't know why I used sprintf (And even only on those cases). What you propose is much better, and it gets rid of the undefined behaviour of sprintf.

Honestly I never thought anyone would pay attention to my little program, much less look at the code. So, thank you very much for your kind feedback. I'm modifying it :wink:

Greetings, Juan.

Thomas-- commented 4 years ago

Thanks for the fast reaction. And thanks for the program, it's pretty useful for creating presentation slides.