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

LOADB/SAVEB hang on invalid device #499

Closed
jkotlinski opened this issue Dec 27, 2022 · 27 comments · Fixed by #501
Closed

LOADB/SAVEB hang on invalid device #499

jkotlinski opened this issue Dec 27, 2022 · 27 comments · Fixed by #501

Comments

@jkotlinski
Copy link
Owner

12 device parse-name foo $7000 loadb <-- hangs

12 device $7000 $7100 parse-name foo saveb <-- hangs

@Whammo
Copy link
Collaborator

Whammo commented Dec 27, 2022

IEC commands do not freeze on invalid device.
#482

 code ds
      w stx,
    $00 lda,#
    $90 sta, \ clear st
    $ba lda,
  $ffb1 jsr, \ listen
    $6f lda,#
  $ff93 jsr, \ second #15
  $ffae jsr, \ unlisten
    $90 lda, \ readst
+branch bne, \ device not present?
\ checking for the device is optional
\ but a good idea.
    $ba lda,
  $ffb4 jsr, \ talk #8
    $6f lda,#
  $ff96 jsr, \ tksa #15
here
    $90 lda, \ readst
+branch bne,
  $ffa5 jsr, \ acptr get byte
  $ffd2 jsr, \ chrout
swap    jmp,  \ loop
:+
  $ffab jsr, \ untalk
:+
      w ldx, ;code

@Whammo
Copy link
Collaborator

Whammo commented Dec 27, 2022

Which means DOS will also freeze when device is invalid.

@Whammo
Copy link
Collaborator

Whammo commented Dec 27, 2022

It's not load or save that freezes, it's open.

@Whammo
Copy link
Collaborator

Whammo commented Dec 27, 2022

It might be simpler to vector kernal open through something like this:

        LDA #$00
        STA $90       ; clear STATUS flags

        LDA $BA       ; device number
        JSR $FFB1     ; call LISTEN
        LDA #$6F      ; secondary address 15 (command channel)
        JSR $FF93     ; call SECLSN (SECOND)
        JSR $FFAE     ; call UNLSN
        LDA $90       ; get STATUS flags
        BNE .devnp    ; device not present
        JMP open
.devnp
        ... device not present handling ...
        RTS

@jkotlinski
Copy link
Owner Author

jkotlinski commented Dec 27, 2022

The original code for reading error channel is here: https://codebase64.org/doku.php?id=base:reading_the_error_channel_of_a_disk_drive

" A small warning: Both the BASIC and the Assembler versions will deadlock if the device is not present. "

Edit: It also presents an alternate version which avoids the deadlock.

Edit 2: OK, it looks like you found the same code :-)

jkotlinski added a commit that referenced this issue Dec 28, 2022
@jkotlinski
Copy link
Owner Author

OK, I think the above commit might work.

The codebase64 page says:

"In this version the status-byte is accessed directly which reduces the portability of the code." <-- That's not great, but OK...

"This will limit you to IEC-bus devices" I am not sure what practical consequences that has. Would one ever want to LOADB/SAVEB to something else?

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

You can use kernal readst, but unlike basic, it doesn't clear after reading.
EOF (st=64) isn't really an error is it?
Save to cassette??

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

We have to reset status before IEC commands because they only change ST on failure.

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

Yes, DOS, INCLUDED and OPEN, and anything with kernal open is subject to hang on device not present.

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

Another advantage of IEC commands is they don't consume file numbers.

@jkotlinski
Copy link
Owner Author

jkotlinski commented Dec 28, 2022

When testing with #501, these cases produce error messages and no hang:

12 device parse-name foo included

12 device dos scratch0:test

Maybe everything is fine with #501? Am I missing something?

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

12 device dos
no string freezes
12 device dos i
okay. workaround?
INCLUDED working correctly is good news.

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

The problem is a bug with the Kernal opening a file with no file name.
It creates the file entries in the tables so chkin doesn't error , then exits.
The command channel doesn't require an (IEC) open or a close,
so it doesn't check if the device is there before it goes to chrin, (acptr) then it gets stuck waiting.
One solution is to never close 15.

@jkotlinski
Copy link
Owner Author

jkotlinski commented Dec 28, 2022 via email

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

DOS with an empty string prints the current device status if it's there, and hangs otherwise.
This is why BASIC insists upon device numbers for anything other than tape.

@jkotlinski
Copy link
Owner Author

I see, so it is a use case.
Maybe send-cmd should be rewritten, too, to use IEC instead of OPEN?

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

: send-cmd ( addr len -- )
?dup if
$f $f open ioabort 
clrchn $f chkin ioabort
begin chrin emit readst until
clrchn $f close cr
else drop _errorchread then ;

perhaps?

But yes it's doable.

@jkotlinski
Copy link
Owner Author

That seems like a fine solution. Added!

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

I think there were some additions to V that can be rolled back now.

jkotlinski added a commit that referenced this issue Dec 28, 2022
* fix LOADB/SAVEB hang on device not present

closes #499

* fix DOS hang on device not present
@jkotlinski
Copy link
Owner Author

Oh really? What V additions?

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

added v check for bad device# ?

@jkotlinski
Copy link
Owner Author

ah right! great catch. thank you!

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

At some point in the future maybe, we can get durexForth off of the filesystem and onto IEC commands so file numbers used by the system would go away.

@jkotlinski
Copy link
Owner Author

That stuff is a bit above my head probably :-)

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

Serial IEC is certainly arcane. The documentation is sparse. The IEEE parallel PET stuff is well documented, of which the serial stuff is a tiny subset, the overview makes it understandable.

@Whammo
Copy link
Collaborator

Whammo commented Dec 28, 2022

Wow! Thanks. I'll try not to f- anything up. 🥇

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants