Bug in the check for end of file on core file reading

[woj76]

This line of code:

until (val > pmax_config.max_address) or (IOResult = 3);

(a) should have "val >= pmax_config.max_address" because (b) "IOResult = 3" is specific to MyDOS. The effect is that on DOSes (I tried with FATFMS/SIDE3) that do not report this code, the program tries to read the core.bin file beyond the end of it, this results in progress 101% and an error message.

[MADRAFi]

thanks, fixed in v1.1

[woj76]

You may want to revert this commit and remove the release, I just bricked two PokeyMAX 4s with the new version, and it did work before (even though there was an error message). At first I thought it was because of the NTSC system, but the second one was PAL. It is not a problem for me, I have the JTAG cable and software, but it might be for others. (Looking at the code I still do not know why this happens, I will check again, maybe it is unrelated...).

EDIT: I looked at the code again, I am really not sure if the bugfix is at fault. I cannot test today anymore (JTAG cable is at another place) if the old version also breaks it, will do that tomorrow.

EDIT2: What is also really weird is that despite a one condition / character change, the XEX files for version 1.0 and 1.1 differ by ~400 bytes. I know compression and all, but that should not be the case?

[MADRAFi]

that should be resolved, please test.

[woj76]

I will tomorrow hopefully (need to resurrect the Pokeys first), in the meantime I made sure I can compile the tool myself with MP to do some experimentation, this actually revealed a bug in MP (also reported) :/ While at it, I looked more at your code, I can see some dangerous things there, that I saw in action when using the tool today. For example, in UpdateProgress you re-use file_version to print the XX%, version 1.1 when finishing the (failed) flashing the branch of the code doing pm_version := file_version displayed 97% (last time when the progress updated due to needing val % 10 == 0) in the core version field. And I still do not understand why the fix that I proposed with "val >=" breaks this, it should work, and why you need to try reading beyond the end of file. I also bet that your new 1.1 will work, and display max 100%, but still report file reading error (on SIDE3/FATFMS), if it will go this way, then there is something seriously wrong about me understanding your code... ;)

[woj76]

OK, so all my hunches were confirmed. First, I bricked the chip with yesterday's first 1.1 with the fix that I suggested because the core.bin file was corrupted / wrong (I still do not know how, I swear I flashed it before with foft's tool and it worked). With the correct core.bin file the first 1.1 version from yesterday actually does work, and works clean - gets to 100%, no "file reading error" messages, and the chip works. With the second redone 1.1 (with checking for i > 100), it also flashes OK, ends with 100%, but I do get "file reading error" message (as I suspected, it still attempts to read beyond the end of file). So the fix to change "val >" into "val >=" is in fact correct, you can bring it back. And then the "if i > 100 then i := 100" is not really needed.

But then there is another glitch with the first 1.1 version from yesterday that I now confirmed to work. If I flash first, and get a clean ending as described above, the code branch:

pm_version := file_version;

gets executed and the displayed file version on the screen is "98%". You cannot reuse file_version variable for temporary storage in UpdateProgress, but from what I can tell, you can reuse core_file one without any problem.

[MADRAFi]

fixed in 1.2. Please test flashing from regular floppy with mydos and sparta dos

[woj76]

Tested on a 130XE PAL U1MB machine:

1. MyDOS floppy booted from SDrive - flashing and verification works fine
2. SDX booted off U1MB with AVGCart/SIDE2 mounted HDD and SDX file system - flashing and verification works fine
3. SIDE3 booted as a cart (not as U1MB integrated HDD) and FATFMS enabled - flashing and verification works fine

Also, there was no visual glitches, all versioning information was correctly updated on the screen.

What did not work, at all, on the attempt of reading the core.bin file the program crashed and exited back to the loader - trying to load the program from the "native" U1MB loader with AVGCart in SIDE2 mode, and the said loader having FATFMS enabled (in this case it is supposed to be read-only anyhow). This, however, does not work with any of the pm_update versions, so something else is at fault (I am guessing a memory conflict between the MP generated binary and the FATFMS loader, can't think of anything else). I generally had problems with the AVG cart working in SIDE2 mode in all cases when the computer was not directly booted into HDD enabled U1MB SDX. So this case can be ignored, but perhaps fjc should know about this, IDK...

The Pokey is PokeyMax 4 with M08, and the only problem with the test is that I was re-flashing the same core, so I am only sure that the whole core is flashed by careful analysis of your code, with my experience I am certain all data gets flashed. Otherwise, this line in the code is totally redundant:

if i > 100 then i := 100;

I am sure it never gets to 100% in UpdateProgress and it is called last time before the last block is read from the disk, the 100% message comes from the finishing procedure.

Another redundant thing is the "break;" at line 498.

But essentially the bug can be close, thanks a lot for being very responsive, and very sorry for the false alarm with flashing the corrupted core (I still need to investigate why it was corrupted).