I took a more detailed look at the code, and found a few things
You have a bug here, whenever the input filename has an extension shorter then 3 chars
Code: Select all
output_name= (char *)malloc(strlen(argv[1])+1);
ext[0]= 0;
strcpy(output_name, argv[1]);
strcat(output_name, ".TAP");
This will fix it:
Code: Select all
ext[0]= 0;
output_name= (char *)malloc(strlen(argv[1])+4+1); // strlen(argv[1]) + strlen((".TAP") + 1;
strcpy(output_name, argv[1]);
strcat(output_name, ".TAP");
It's also good practice to check if
malloc returns something != NULL, but I leave that to you
Code: Select all
unsigned char mem[0x10000]
...
size= fread(mem+23, 1, 0x10000, fi);
Previous line is a potential bug, because it will do a buffer overrun if the file is larger than 0x10000 or 65536 (actually a little less, 24 bytes will do).
Overrun happens, if it copies up to the max (0x10000) buffer size, while starting at a larger offset then zero (mem+23).
We can assume the files should be shorter, since the block or header length fields will not allow it, but a user can throw any file at you.
A buffer overrun will surely mess with other application variables on top, and it's nice to not blow up
.
This will only work if we compile the code in a
little endian CPU architecture.
When outputting to files, it's advisable to handle byte ordering, and not rely on type of CPU architecture of host CPU matching the one of the target computer.
Although if building for Windows (.exe), you might not have considered this could be built in other OS/CPU architecture.
On another note, since an
int is 4 bytes, it's also initializing the
flag and
first header byte to zero, meaning respectively an
header block of a
program. (sneaky coding
).
Code: Select all
*(short*)(mem+18)= *(short*)(mem+14)= *(short*)(mem+21)= size+20;
Apparently here is another bug, since according to TAP format
https://faqwiki.zxnet.co.uk/wiki/TAP_format
at position
mem+14 ( 2 +1 +1 + 10), is the position of the header
Length of data block which should be 2 bytes less than the actual next code
Block Length (
mem+21) since it does not include the block flag or checksum bytes.
Most emulators (eventually ZX ROM too) will probably only consider the next
Block Length to determine the program size, but the
Length of data block in header and actual code
Block Length will be inconsistent if left like this.
The reminder of the code is a bit hard to follow due to "Magic Numbers"
, that I presume is a BASIC program.
Code: Select all
*(int*)(mem+3)= 0x8e117d06;
*(int*)(mem+7)= 0x0e37c0fd;
*(int*)(mem+11)= 0x2196398f;
*(int*)(mem+15)= 0xb8edda6d;
*(int*)(mem+19)= 0xe9f9eb13;
I would suggest, that you add some comments and refactor parts of the code into functions (calc_checksum, initHeader, writeBlock, etc..), so that it's easier to maintain.