I have a big problem with opening my own chess games, scid crashes during opening the database. :( I tried scid stable from jessie and today compiled from sources the lates git branch. Nothing happened. :(
Because nobody did help, I have fixed your database, although I'm not the maintainer nor the developer of Scid.
I've attached the fixed base, was a diffcult fix, Scid has written a defect namebase, and one of the errors is a wrong number of name entries for the player names. With my fix one of the player names is lost (black player in game #63).
It seems that your database had any special property which caused the corrupted namebase, therefore I recommend to make backups in the next time before working on this database, it's likely that this bug occurs again.
NB: the crash occurs because in namebase.cpp after "fclose(FilePtr)" the statement "FilePtr = NULL" is missing. After this correction Scid is not crashing, instead an error message, that the namebase is corrupted, will be displayed (when opening the corrupted database).
1) thank you for helping me and fixing my games database. :-) Ineed I think that this bug is still present in scid because I two times 'incidentally' broke my database.
2) name of the black player in #63 is not a problem I remember that game :)
3) in namebase.cpp I don't see any fclose(FilePtr), please tell me exactly which line number and branch are you used to fix my db.
Regards and again thank you very much,
Karol
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
1) I think that this bug is still present in scid because I two times 'incidentally' broke my database.
3) in namebase.cpp I don't see any fclose(FilePtr), please tell me exactly which line number and branch are you used to fix my db.
I was wrong with my hint (to the developer) that "FilePtr = NULL" should be added, because this belongs to Scid vs PC. I've used this derivate of Scid for fixing the database. But now I see that mainline Scid (you are using mainline Scid) has changed this file, it's a completely new implementation of namebase.cpp. Because this bug with your database is new, I think that the new implementation has introduced this bug. The derivate Scid vs PC is using the original implementation, and with original implementation a bug like this with the namebase is not known (it only has the problem that it is crashing if the database is corrupted). So it seems that newer versions of Scid are not stable.
This hint with "FilePtr = NULL" is only preventing a crash when a database with a corrupted namebase (like in your case) has been opened, this is not fixing the base, and I will send a patch to Steve (the maintainer of Scid vs PC), so newer versions should display an error message instead of crashing. For the fix of your database I have done some changes in the code of Scid vs PC, this means I've prepared a special Scid version for the manipulation, and this cannot be explained easily - as I said, it was a bit difficult to fix the database. But I know the Scid code base (the original implementation) very well - I have implemented a few features for Scid vs PC -, otherwise it wouldn't be possible for me to do this.
thank you for helping me and fixing my games database. :-)
Your welcome, anybody should help.
Gregor
Last edit: Gregor Cramer 2016-07-21
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Haters gonna hate.
He said that he updated to the latest version AFTER his database was corrupted.
But, as usual, there should be a ghost bug in the lastest scid code (that you obviously did not even read) and it's better to use the old, messy, slow code of scid vs pc, bla, bla, bla.
I fixed the segfault with commit: https://sourceforge.net/p/scid/code/ci/6f3492dcd01db6912a661bea3314d18ec00dc7aa/
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
You've closed this bug report without any analyis, and you're comment is "...bla, bla, bla"? This is ridiculous!
Here is an analysis that you're new implementation is writing corrupted namebases (after reading your code I could find the problem, because I know the original implementation):
Player A (using a newer Scid version with new implementation of namebase) is creating a new database, and adds new game with White="Ü", and Black="" (please mind the umlaut; you can use any other Unicode character with codepoint > 128 instead). Then he sends this database to Player B.
Player B (using an older Scid version with original implementation of namebase, or Scid vs PC), is opening this database, and adds a new game with White="Ü", and Black="?". Then he sends back the modified database to player A.
Player A opens this database, and adds a new game with White="A", and Black="B". Now this Scid version (with new implementation of namebase) is writing a corrupted database.
This is an interplay based on a namebase denormalization in original Scid implementation, but:
It is not possible to produce this denormalization with original implementation, because this requires that you are inserting an empty player name, and this is not possible with original Scid implementation, it is automatically converting empty string to "?". So you need the interplay with new implementation to produce this denormalization.
The original implementation is not writing corrupted namebases, if you do the third step with original implementation (adding game with White="A", and Black="B"), the namebase is not corrupt, it is only denormalized, but new implemention is writing a defective namebase. (Both versions are able to open denormalized namebases, but both versions are not able to open the defective namebase. The original implementation can handle denormalization, but not the new implementation - it writes defective namebases -, although new implementation is the causer.)
It's true that I don't like mainline Scid, it's obvious why. A serious developer is doing a careful analysis. It's not really a problem that you have not known this (suprising) oddity of original implementation, but it's a problem that you are ignoring problems with corrupted databases. The result is a loss of reliance in mainline Scid. I like to emphasize that you're new implementation has introduced the novelty with empty namebase strings, this is not belonging to .sn4 standard, because you cannot write empty namebase strings with original implementation (the father of the standard).
So I have to correct the statement "...it seems that newer versions of Scid are not stable" into: Newer versions of mainline Scid are violating the standard and producing defective databases.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
The bug tracker exists with the only purpose to help Scid development.
IMHO the previous comments did not include anything that could do that, and that's why I closed the bug report.
Your latest comment instead is a perfect bug report, with a procedure to reproduce the bug and even an analysis of the cause.
That will help Scid development and I thank you for that.
But I still believe that, with only the information provided before, closing the bug report was the right thing to do.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Fulvio, without help from Gregor I would have still broken database and lost all of my games.
And secondly it hard to expect from Scid users that during filling bug report they show exactly line number of the source file where the bug exists.
Thanks and regards, Karol
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I think that "a namebase denormalization in original Scid implementation" is slightly suspicious.
After a quick look it appears to me that the namebase file created by the old code have 2 big problems:
contains duplicate entries ("Ü" have both id 0 and id 2)
it's not alphabetically sorted
I have pushed a commit that rejects such file: https://sourceforge.net/p/scid/code/ci/7a6dcdf8131b952609609b660233875e8964c224/
because it can indeed create unpredictable side-effects.
I'll have a deeper look at the old code (in the meantime "denormalized namebases" can be corrected copying the games to a new database)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Gregor Cramer wrote:
"I like to emphasize that you're new implementation has introduced the novelty with empty namebase strings, this is not belonging to .sn4 standard, because you cannot write empty namebase strings with original implementation (the father of the standard)"
This is not true.
I have checked Scid v4.5.2 (the latest with the old code) and ScidVsPC v4.16.1 and they both can import pgn with empty names in a .sn4 file
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
I have checked Scid v4.5.2 (the latest with the old code) and ScidVsPC v4.16.1 and they both can import pgn with empty names in a .sn4 file
This is a bug in PGN import and has to be fixed, I will report this bug to Steve. Fact is that original Scid cannot handle empty strings in namebases correctly, and therefore it's not allowed to insert empty strings into namebase. PGN import of Scid has some severe bugs, not only this one. If you have a look into strtree.h you will see the assertion ASSERT((byte)(Root[i]->name)), this assertion "prevents" (if enabled) the insertion of empty strings (indirectly, the build of the tree fails).
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
All the Scid version till now allowed to import empty strings: there are probably a lot of databases around with empty string and i'm not sure if it's the problem.
Another possibility is that the StrTree class used by the old code was not designed to deal with unicode chars.
For example creating a database with 2 games:
white "AA" - black "AÜ"
white "ÜA" - black "ÜÜ"
will write a namabase with unsorted names ("AÜ" "AA" "ÜÜ" "ÜA").
Are you aware of any problem with a database that include empty strings but not unicode chars?
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
All the Scid version till now allowed to import empty strings: there are
probably a lot of databases around with empty string and i'm not sure if
it's the problem.
It seems that only the "denormalization" problem exists, old Scid works without any problem with denormalized namebases. I've suggested to Steve to print a warning when opening a database with denormalization, so the user can repair the namebase (or he can make a copy).
Are you aware of any problem with a database that include empty strings but
not unicode chars?
No, if no Unicode characters exists then no denormalization will happen. But as soon as a name starting with a Unicode character will be added, the namebase is denormalized.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
old Scid works without any problem with denormalized namebases.
A NameBase with duplicate entries seems very scary too me.
Does name editing works? Name spelling? Twin games?
I think that a lot of testing is necessary before saying for sure that it works without any problem...
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Gregor Cramer wrote:
If you have a look into strtree.h you will see the assertion ASSERT((byte)(Root[i]->name)), this assertion "prevents" (if enabled) the insertion of empty strings (indirectly, the build of the tree fails).
It is also important to remember that the NameBase class is used for all the memory/imported databases, not only si4 ones.
If we follow your logic this means that Scid should not handle empty strings with any type of current or future database formats (si4, pgn, clipbase, etc...)
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Line 379, This assertion fails as soon the following is satisfied: 1. empty string 2. string starting with char ≥ 128.
If we follow your logic this means that Scid should not handle empty strings with any type of current or future database formats (si4, pgn, clipbase, etc...)
Yes, for me this is also surprising, and I've already made a fix in export to .si4 (Scidb), not writing empty strings anymore. For any reason Shane Hudson has decided to convert empty strings automatically to "?" (to be PGN conform?), and I think his implementation of strtree is buggy, he has overlooked the case with empty strings, because he didn't encounter empty strings in his tests (I guess). This is really nasty.
BTW: When importing .si4 (in Scidb) I do an automatic conversion from "?" to an empty string. IMO this mapping in Scid from empty string to "?" is a PGN reminiscence, and PGN is misused, in general not adequate for the transfer of chess games. Due to Wikipedia the purpose of PGN is "a plain text computer-processible format for recording chess games", but not more. I'm still working on C/CIF, the first format for losless chess data interchange.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
A NameBase with duplicate entries seems very scary too me.
Does name editing works? Name spelling? Twin games?
I think that a lot of testing is necessary before saying for sure that it works without any problem...
You're right, my assumption is based on quick tests, not more. But my "feeling" says that no bad things will happen. Of course, a denormalized namebase should be avoided.
If you would like to refer to this comment somewhere else in this project, copy and paste the following link:
Attached database.
karol@debian:~/Dokumenty/chess/scid$ scidt -i mygames
Database "mygames":
204 games
Version: 4.0
Namebase: 130 players, 52 events, 17 sites, 15 rounds.
karol@debian:~/Dokumenty/chess/scid$
but
karol@debian:~/Dokumenty/chess/scid$ scidt -l mygames
scidt: ERROR: corrupt data reading "mygames.sn4"
karol@debian:~/Dokumenty/chess/scid$
You have forgotten to attach the file mygames.sn4, otherwise nobody can test your database.
Hello Gregor, you right, here is....
and rest of the files.
Because nobody did help, I have fixed your database, although I'm not the maintainer nor the developer of Scid.
I've attached the fixed base, was a diffcult fix, Scid has written a defect namebase, and one of the errors is a wrong number of name entries for the player names. With my fix one of the player names is lost (black player in game #63).
It seems that your database had any special property which caused the corrupted namebase, therefore I recommend to make backups in the next time before working on this database, it's likely that this bug occurs again.
NB: the crash occurs because in namebase.cpp after "fclose(FilePtr)" the statement "FilePtr = NULL" is missing. After this correction Scid is not crashing, instead an error message, that the namebase is corrupted, will be displayed (when opening the corrupted database).
Last edit: Gregor Cramer 2016-07-21
Hi Gregor,
1) thank you for helping me and fixing my games database. :-) Ineed I think that this bug is still present in scid because I two times 'incidentally' broke my database.
2) name of the black player in #63 is not a problem I remember that game :)
3) in namebase.cpp I don't see any fclose(FilePtr), please tell me exactly which line number and branch are you used to fix my db.
Regards and again thank you very much,
Karol
I was wrong with my hint (to the developer) that "FilePtr = NULL" should be added, because this belongs to Scid vs PC. I've used this derivate of Scid for fixing the database. But now I see that mainline Scid (you are using mainline Scid) has changed this file, it's a completely new implementation of namebase.cpp. Because this bug with your database is new, I think that the new implementation has introduced this bug. The derivate Scid vs PC is using the original implementation, and with original implementation a bug like this with the namebase is not known (it only has the problem that it is crashing if the database is corrupted). So it seems that newer versions of Scid are not stable.
This hint with "FilePtr = NULL" is only preventing a crash when a database with a corrupted namebase (like in your case) has been opened, this is not fixing the base, and I will send a patch to Steve (the maintainer of Scid vs PC), so newer versions should display an error message instead of crashing. For the fix of your database I have done some changes in the code of Scid vs PC, this means I've prepared a special Scid version for the manipulation, and this cannot be explained easily - as I said, it was a bit difficult to fix the database. But I know the Scid code base (the original implementation) very well - I have implemented a few features for Scid vs PC -, otherwise it wouldn't be possible for me to do this.
Your welcome, anybody should help.
Gregor
Last edit: Gregor Cramer 2016-07-21
Haters gonna hate.
He said that he updated to the latest version AFTER his database was corrupted.
But, as usual, there should be a ghost bug in the lastest scid code (that you obviously did not even read) and it's better to use the old, messy, slow code of scid vs pc, bla, bla, bla.
I fixed the segfault with commit:
https://sourceforge.net/p/scid/code/ci/6f3492dcd01db6912a661bea3314d18ec00dc7aa/
You've closed this bug report without any analyis, and you're comment is "...bla, bla, bla"? This is ridiculous!
Here is an analysis that you're new implementation is writing corrupted namebases (after reading your code I could find the problem, because I know the original implementation):
Player A (using a newer Scid version with new implementation of namebase) is creating a new database, and adds new game with White="Ü", and Black="" (please mind the umlaut; you can use any other Unicode character with codepoint > 128 instead). Then he sends this database to Player B.
Player B (using an older Scid version with original implementation of namebase, or Scid vs PC), is opening this database, and adds a new game with White="Ü", and Black="?". Then he sends back the modified database to player A.
Player A opens this database, and adds a new game with White="A", and Black="B". Now this Scid version (with new implementation of namebase) is writing a corrupted database.
This is an interplay based on a namebase denormalization in original Scid implementation, but:
It is not possible to produce this denormalization with original implementation, because this requires that you are inserting an empty player name, and this is not possible with original Scid implementation, it is automatically converting empty string to "?". So you need the interplay with new implementation to produce this denormalization.
The original implementation is not writing corrupted namebases, if you do the third step with original implementation (adding game with White="A", and Black="B"), the namebase is not corrupt, it is only denormalized, but new implemention is writing a defective namebase. (Both versions are able to open denormalized namebases, but both versions are not able to open the defective namebase. The original implementation can handle denormalization, but not the new implementation - it writes defective namebases -, although new implementation is the causer.)
It's true that I don't like mainline Scid, it's obvious why. A serious developer is doing a careful analysis. It's not really a problem that you have not known this (suprising) oddity of original implementation, but it's a problem that you are ignoring problems with corrupted databases. The result is a loss of reliance in mainline Scid. I like to emphasize that you're new implementation has introduced the novelty with empty namebase strings, this is not belonging to .sn4 standard, because you cannot write empty namebase strings with original implementation (the father of the standard).
So I have to correct the statement "...it seems that newer versions of Scid are not stable" into: Newer versions of mainline Scid are violating the standard and producing defective databases.
The bug tracker exists with the only purpose to help Scid development.
IMHO the previous comments did not include anything that could do that, and that's why I closed the bug report.
Your latest comment instead is a perfect bug report, with a procedure to reproduce the bug and even an analysis of the cause.
That will help Scid development and I thank you for that.
But I still believe that, with only the information provided before, closing the bug report was the right thing to do.
Fulvio, without help from Gregor I would have still broken database and lost all of my games.
And secondly it hard to expect from Scid users that during filling bug report they show exactly line number of the source file where the bug exists.
Thanks and regards, Karol
I think that "a namebase denormalization in original Scid implementation" is slightly suspicious.
After a quick look it appears to me that the namebase file created by the old code have 2 big problems:
I have pushed a commit that rejects such file:
https://sourceforge.net/p/scid/code/ci/7a6dcdf8131b952609609b660233875e8964c224/
because it can indeed create unpredictable side-effects.
I'll have a deeper look at the old code (in the meantime "denormalized namebases" can be corrected copying the games to a new database)
Hello All,
Thanks for helping me. I would like to notice that corrupting database occured when I used scid from stable version of debian jessie:
scid 1:4.5.1.cvs2 i386
so maybe it will be good to merge fixes into this branch and pushing to debian developers.
and my little notice to scid developers:
in my opinion it will be good to adding to commit log comment number of ticket, for example #102:
Author: Fulvio fbf@libero.it
Date: Fri Jul 22 00:44:49 2016 +0200
Gregor Cramer wrote:
"I like to emphasize that you're new implementation has introduced the novelty with empty namebase strings, this is not belonging to .sn4 standard, because you cannot write empty namebase strings with original implementation (the father of the standard)"
This is not true.
I have checked Scid v4.5.2 (the latest with the old code) and ScidVsPC v4.16.1 and they both can import pgn with empty names in a .sn4 file
This is a bug in PGN import and has to be fixed, I will report this bug to Steve. Fact is that original Scid cannot handle empty strings in namebases correctly, and therefore it's not allowed to insert empty strings into namebase. PGN import of Scid has some severe bugs, not only this one. If you have a look into strtree.h you will see the assertion ASSERT((byte)(Root[i]->name)), this assertion "prevents" (if enabled) the insertion of empty strings (indirectly, the build of the tree fails).
All the Scid version till now allowed to import empty strings: there are probably a lot of databases around with empty string and i'm not sure if it's the problem.
Another possibility is that the StrTree class used by the old code was not designed to deal with unicode chars.
For example creating a database with 2 games:
white "AA" - black "AÜ"
white "ÜA" - black "ÜÜ"
will write a namabase with unsorted names ("AÜ" "AA" "ÜÜ" "ÜA").
Are you aware of any problem with a database that include empty strings but not unicode chars?
It seems that only the "denormalization" problem exists, old Scid works without any problem with denormalized namebases. I've suggested to Steve to print a warning when opening a database with denormalization, so the user can repair the namebase (or he can make a copy).
No, if no Unicode characters exists then no denormalization will happen. But as soon as a name starting with a Unicode character will be added, the namebase is denormalized.
A NameBase with duplicate entries seems very scary too me.
Does name editing works? Name spelling? Twin games?
I think that a lot of testing is necessary before saying for sure that it works without any problem...
I cannot find the line you mentioned:
https://sourceforge.net/p/scid/code/ci/v4.5.2/tree/src/strtree.h#l377
It is also important to remember that the NameBase class is used for all the memory/imported databases, not only si4 ones.
If we follow your logic this means that Scid should not handle empty strings with any type of current or future database formats (si4, pgn, clipbase, etc...)
Line 379, This assertion fails as soon the following is satisfied: 1. empty string 2. string starting with char ≥ 128.
Yes, for me this is also surprising, and I've already made a fix in export to .si4 (Scidb), not writing empty strings anymore. For any reason Shane Hudson has decided to convert empty strings automatically to "?" (to be PGN conform?), and I think his implementation of strtree is buggy, he has overlooked the case with empty strings, because he didn't encounter empty strings in his tests (I guess). This is really nasty.
BTW: When importing .si4 (in Scidb) I do an automatic conversion from "?" to an empty string. IMO this mapping in Scid from empty string to "?" is a PGN reminiscence, and PGN is misused, in general not adequate for the transfer of chess games. Due to Wikipedia the purpose of PGN is "a plain text computer-processible format for recording chess games", but not more. I'm still working on C/CIF, the first format for losless chess data interchange.
You're right, my assumption is based on quick tests, not more. But my "feeling" says that no bad things will happen. Of course, a denormalized namebase should be avoided.