GideonZ
Crash in conversion from G64 to D64
Load the attached G64 image in the 1541 emulation. Try F5, Drive A, Save binary and enter filename. The Ultimate will crash and not even responding to ethernet ping.
It turns out that the content of track 36 in the image causes the crash somehow - so the "Track of Death" exists and has been found.
I can confirm it. Out of curiosity, I compared several g64s. Created and then formatted with N:DATA,OK command.
ulti.g64 -created on ultimate ,no problem ulti format.g64 -formatted on ultimate ,no problem
trackOfDeath.g64 -created on ?? ,crash during "Save as Binary" trackOfDeath.g64 -formatted on ultimate ,crash during "Save as Binary"
vice.g64 -created on Vice ,no problem vice format.g64 -formatted on Vice ,crash during "Save as Binary"
If g64 created on Vice I format it in Ultimate then there is no problem with "Save as Binary"
Can confirm the track of death. Will investigate. It seems that the conversion loop gets stuck in finding sector headers. I think the algorithm needs a bit of refactoring. It seems that there is an unexpected order of events that the algorithm chokes on, e.g. two sector headers in a row without data header? Something like that.
Can confirm the track of death. Will investigate. It seems that the conversion loop gets stuck in finding sector headers. I think the algorithm needs a bit of refactoring. It seems that there is an unexpected order of events that the algorithm chokes on, e.g. two sector headers in a row without data header? Something like that.
There are tracks, like in pirateslayer protection, that have no sync at all. Also "unformatted" tracks have no syncs and they are present in many game images.
I have isolated the disk_image code and compiled it on a PC, so that I could turn this into a test case. I am embarrassed to see how tangled the code was with user interface stuff. This calls for a thorough refactoring.
Anyhow, I saw the problem. Track 36 of this EMPTY DISK (!!) is just full of crap.. The headers after the sync are of invalid type. The real problem is in the fact that the routine does not properly detect a revolution. The algorithm is supposed to stop searching after two revolutions, or when the number of expected sectors is reached. This never happens, because a variable is used twice, for two different purposes. There is a simple fix, but I'll include it when the whole code is refactored.
Yes, that disk image is empty. For the purpose of this issue, the content of tracks 1 to 35 do not metter, so I replaced them with empty disk's content. The track 36 is a track I found on one of my disk images - very probable read from an original disk with zoomfloppy or kryoflux or so.
Sine @radius75 observed that crash in images from VICE, it seems to be more common than expcted.
If you think it's a good idea, I can make a hot fix on the current yucky code... It's pretty simple. The question remains: what would you like the output to be? Since no sectors will be found on track 36, the binary will be just 35 tracks, with no errors.
In case you have no problem that I prefer testing with https://github.com/GideonZ/1541ultimate/pull/329 and https://github.com/GideonZ/1541ultimate/pull/294, I'll test a hotfix with joy.
The question remains: what would you like the output to be? Since no sectors will be found on track 36, the binary will be just 35 tracks, with no errors.
I think if a track i has no data, you should distinguish two cases:
(1) There is a track k (k > i) that has data: Then the track missing track data has to be added as a filler, 0x"2bad" might be a filler that helps the human reader recognising it. (2) There is no such track: Fill it if i ≤ 35, otherwise ignore the track.
For first tests, we might just ignore such tracks, but that's not correct.
So basically, we initialize the BinImage as 35 tracks, but reserve space for 41 tracks (for instance), just set the "last valid track" to 35.
Then fill the buffer with '2bad' (haha)
For all tracks: decode. If data found and (track > last valid track) => last valid track := track.
Message ID: @.***>
Right. In case there were decode errrors that are to be saved inside the image, the extension with error bytes for each sector is used.
BTW: The size of each D64 or D71 has to be a multiple of 256 or 257 bytes. The former for a regular D64, the latter for a D64 with error information.
And since 256*257 bytes is too short for a D64 image and twice that value is too long for a D64, testing length modulo 256 or 257 will tell you if the image is pausible and if it has error information or not. D71 likewise, divisable by both 256 and 257 cannot happen for a real image.
BTW: I havn't seen a D71 with k ≠ 70 Tracks yet - and no speeder that uses more than 35 tracks each side on a 1571 for a double sided disk.
Oh good, because the Ultimate does not support D71's with any other than 70 tracks either.
I've quickly tested your fix. Well, it does not crash anymore, that's really good.
But @radius75 's test, creating a g64 in vice and then formatting it in vice still yields a g64 that the Ultimate does not process right. The following example has been created that way. empty-vice.zip Note that the Ultimate saves it as as 11k d64 - well, I don't have to comment that.
https://github.com/GideonZ/1541ultimate/pull/294 helps, only one decoding error is still leaving. g64conv verify test.d64 shows that there are errors in the error section of the saved d64:
Track 1 sector 0 should be 1, not 0. Track 1 sector 1 should be 1, not 0. Track 1 sector 2 should be 1, not 0. Track 1 sector 3 should be 1, not 0. Track 1 sector 4 should be 1, not 0.
and so on... Someone decided that no error is error code 1, not 0.