Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

DOS unit tests 07 and 08 (C: copy/concatenate) are broken #350

Open
mist64 opened this issue Oct 26, 2022 · 4 comments
Open

DOS unit tests 07 and 08 (C: copy/concatenate) are broken #350

mist64 opened this issue Oct 26, 2022 · 4 comments
Labels
bug Something isn't working [DOS] Part of CMDR-DOS

Comments

@mist64
Copy link
Collaborator

mist64 commented Oct 26, 2022

This breaks in tests 07 and 08 (C: COPY FILE/C: CONCATENATE FILES):
unzip -o sdcard.img.zip
./x16emu -sdcard sdcard.img -warp -bas ../x16-rom/test/dos.bas

It reproduces also when disabling all other tests.

This has been broken since at least r39. Bisecting older ROMs needs more work because it needs an older emulator.

@mist64 mist64 added bug Something isn't working [DOS] Part of CMDR-DOS labels Oct 26, 2022
@mist64 mist64 changed the title DOS unit text 07 is broken DOS unit tests 07 and 08 are broken Oct 26, 2022
@mist64 mist64 changed the title DOS unit tests 07 and 08 are broken DOS unit tests 07 and 08 (C: COPY) are broken Oct 26, 2022
@mist64 mist64 changed the title DOS unit tests 07 and 08 (C: COPY) are broken DOS unit tests 07 and 08 (C: copy/concatenate) are broken Oct 26, 2022
@Jaxartes
Copy link
Contributor

Jaxartes commented Nov 1, 2022

The first ROM commit to have the problem seems to be b6a0f0c.
Other observations (running on that commit):
In test 7, the file contents are "FILE1" when they should be "HELLO, WORLD!".
In test 8, the file contents of "FILE4" are "FILE1".

@stefan-b-jakobsson
Copy link
Contributor

I think I now know what's happening when doing a DOS file copy to some extent.

AFAICT, the actual file copying is done by fat32_read and fat32_write in dos/fat32/fat32.s.

It seems fat32_read uses a temporary storage buffer in RAM bank 1 where it puts the content of the file it's reading.

Furthermore it looks like fat32_write tries to output the content of the same buffer to disk, but that it fetches data from the same memory address in RAM bank 0 instead.

Even so, I have not yet a concrete bug fix. Just an idea of what might be going wrong.

@stefan-b-jakobsson
Copy link
Contributor

functions.diff.txt

@stefan-b-jakobsson
Copy link
Contributor

stefan-b-jakobsson commented Nov 10, 2022

I managed to trick the X16 Kernal to do a correct file copy by the changes found in the above diff file "functions.diff.txt"
Those changes basically set the variable bank_save to zero before starting the copy procedure. But this is not the right way to really fix the problem.
@mist64, where do you think we should start looking for a solution?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working [DOS] Part of CMDR-DOS
Projects
None yet
Development

No branches or pull requests

3 participants