Errors when adding some files to a cdt/dsk

[nestornillo]

Adding binary files to a cdt/dsk sometimes fails due to an incorrect identification of a non existing Amsdos header. Most of this cases are caused by zeros at the begining of the file.

an Amsdos header (128 bytes) consists of:

67 bytes:  data about the file
 2 bytes:  sum of previous 67 bytes
59 bytes:  not used by Amsdos

When adding binary files, cpc2cdt iDSK and dskgen try to detect if the file contains an amsdos header. All of them check if the sum of the first 67 bytes agrees with bytes 68 and 69.

If a binary file starts with some zeros: -iDSK if iDSK detects that first 69 bytes are zero, it assumes the file has no valid header, generates a new one, and it works fine.

-cpc2cdt cpc2cdt only checks if all first 128 bytes (full header) are zero for discarding an invalid header. So, if a file starts with 69-127 zeros, cpc2cdt tries to use the zeros as a header, and gives error "short read while reading AMSDOS header" That can be easily fixed changing line 330 of cpc2cdt.c: if ((body[0x43]+256*body[0x44])==j) { // AMSDOS! for if (((body[0x43]+256*body[0x44])==j)&&(j!=0)) { // AMSDOS! (j is the sum of first 67 bytes)

-dskgen dskgen does not check if first bytes are zero, only if the sum is valid. So, if a file starts with 69 zeros, dskgen uses that as a header and gives no errors, but loading of the file fails. That can also be easily fixed changing line 227 of Disk.cc: return checkSumInHeader == checksum; for return ((checkSumInHeader == checksum)&&(checksum!=0));

However

There are more cases of incorrect identification of a non present header, for example, a file starting with:

.db 1
.ds 0x42
.db 1
.db 0

That will produce error "short read while reading AMSDOS header" when using cpc2cdt. Using dskgen and iDSK there are no errors, but loading fails.

Also if a binary starts with, for example:

.db 1
.ds 0x7E
.db 1

cpc2cdt will give error "short read while reading PLUS3DOS headermake". In this format the checksum is only 1 byte long (sum_first_127_bytes mod 256 == byte_128), so it can potentially be more incorrect maches.

If we know that a file doesn't contain an Amsdos header we don't need to check if a header is present (and potentially lead to errors in the process). This is why I have made working small modifications to the three utilities to prevent the search of a header when not needed. For cp2cdt and iDSK I've added an additional optional parameter that skips header check. For dskgen I've added a new type for the header parameter. Should I pull request all/some of this changes?

[lronaldo]

Neat analysis and proper solution. Adding a flag for not checking for AMSDOS header on raw binaries is clearly needed for these cases, to prevent these problems.

Please, add the PR with your modifications. We can have a short review on them and merge with development branch. I think they are greatly useful.

Thank you very much, @nestornillo :)

[nestornillo]

I will place now one PR for each of the three utilities. Later we can see what to change to CPCtelera.