MEGA65 / mega65-rom-public

MEGA65 ROM public issue reporting
4 stars 0 forks source link

LOAD raw mode always returns SUCCESS when loading in raw mode #125

Open ki-bo opened 4 months ago

ki-bo commented 4 months ago

Test Environment (required) You can use MEGA65INFO to retrieve this.

Describe the bug When loading a file in raw mode (bit6 of A is set when calling LOAD at $ffd5), then the error reporting is broken. If the file could not be loaded, it is expected that C is clear, but it is set in any case.

To Reproduce Steps to reproduce the behavior:

  1. Use this program to load a non-existent file "TEST":

    
    lda #$04
    ldx #<filename
    ldy #>filename
    jsr $ffbd       // SETNAM
    lda #$00
    ldx #$08
    ldy #$00
    jsr $ffba       // SETLFS
    lda #$00
    ldx #$00
    jsr $ff6b       // SETBNK
    lda #$40        // bit6=1 is RAW mode, don't skip first two bytes
    ldx #$01
    ldy #$50
    jsr $ffd5       // LOAD
    
    bcc loadok
    lda #2
    sta $d020
    jmp *
    loadok:
    inc $d020
    jmp loadok

filename: .text "TEST"


2. Execute and make sure the file TEST is not on the mounted disk image or floppy disk
3. The border will cycle colours as if the LOAD succeeded

**Expected behavior**
Border should turn red to indicate a LOAD error (carry set when routine returns)

**Additional context**
If changing A to 0 again before calling $FFD5, everything works as expected.
I believe that raw mode is a new feature introduced by the MEGA65 ROM, and probably was not there with C65, as it is not documented. I only learned about it on Discord.
dansanderson commented 3 weeks ago

As it is currently written, it's relying on reading the first two bytes to detect File Not Found, and in fact any issue with reading the first two bytes will be reported as File Not Found. In raw mode, it doesn't read any bytes before proceeding as if the file exists.

We might be able to rewrite this to always read the first two bytes into a temporary spot, FNF on any error (not perfect but probably ok), then write those two bytes to the requested memory address if in raw mode and proceed from start+2. Alas, more design flaw than bug, but still fixable.

dansanderson commented 3 weeks ago

oof, lots of edge cases here, would need a major rewrite to do it that way. backing away from that idea.

Strange because subsequent reading of the non-existent file should also error. The "timeout" call path does set Carry. Somehow this is exiting by another path in this case.

dansanderson commented 3 weeks ago

... When this load happens from an internal disk, it uses CBDOS FastLoad, which does not appear to detect or report errors. It just assumes that previously run code verified that the file exists, and makes no attempt that I can see to report failure to the caller.

As far as I can tell, a non-existent file has the same result as a zero-byte file in this call path. Maybe on load_done we can detect a zero byte file (eal=sal), then in that case try another disk operation to confirm that the file exists and report FNF if it doesn't. That's rare enough that another disk op to confirm wouldn't be burdensome.

Gonna pause this for now. Hopefully this isn't too intrusive in the meantime.