Fix DOS1 mode, page 1 DEV_RW error checking

[S0urceror]

Hi, I propose this PR to fix the DOS1 mode, page 1, DEV_RW return value error checking. See DRV.MAC.

I made a build of Nextor on my Mac and had to make a couple of changes to make it build. Please ignore these if they are not relevant.

PS. great Makefile does a lot of magic!

Closes https://github.com/Konamiman/Nextor/issues/110

[Konamiman]

Hi @S0urceror, thanks for your contribution. May I ask for a few changes:

1. Remove the changes in printf.c, I have already adapted all the C sources to SDCC 4.2 in https://github.com/Konamiman/Nextor/pull/115

2. Remove the export statement in the makefile. This is specific to your machine, and additionally, the makefile for the kernel allows now to specify the location of specific tools via environment variables, e.g. M80=path/to/M80 make.

3. Remove the "fix"/"end fix" comments. This information belongs in the source control system, not in the source code.

4. Add some testing instructions to the pull request: what would be the steps to reproduce the issue that you are fixing, and how to verify that it's actually fixed?

[S0urceror]

I guess you’re right. This is a WR function so the JR should go to WR_OK. The change is checking for Z not C flag.

Sent by phone

---

From: Néstor Soriano @.> Sent: Sunday, April 16, 2023 10:03 PM To: Konamiman/Nextor @.> Cc: S0urceror @.>; Mention @.> Subject: Re: [Konamiman/Nextor] fix DOS1 mode, page 1 DEV_RW error checking (PR #111)

@Konamiman commented on this pull request.

---

In source/kernel/drv.machttps://github.com/Konamiman/Nextor/pull/111#discussion_r1168011478:

@@ -539,7 +542,10 @@ DIO_WR_LOOP: ex af,af' ld a,DV_BANK## call CALBNK##

• jr nc,DIO_WR_OK
• ; fix by S0urceror
• and a
• jr z,DIO_RD_OK

Shouldn't this jump to DIO_WR_OK instead?

— Reply to this email directly, view it on GitHubhttps://github.com/Konamiman/Nextor/pull/111#pullrequestreview-1387035291, or unsubscribehttps://github.com/notifications/unsubscribe-auth/ABGC6KGHD56SYMP5JA3RADDXBRF7TANCNFSM6AAAAAAQJ6KHC4. You are receiving this because you were mentioned.Message ID: @.***>

[Konamiman]

Hi @S0urceror, I'm preparing the release of Nextor 2.1.2. Would you be able to handle the changes I requested in https://github.com/Konamiman/Nextor/pull/111#issuecomment-1509822322 and https://github.com/Konamiman/Nextor/pull/111#discussion_r1168011478 please? Also there are now conflicts in .gitignore and printf.c that need to be solved (feel free to ask me if you don't know how).

[Konamiman]

By the way @S0urceror I've set you as "assignee" for the pull request because it makes it easier for me to see who's the author in the list of open pull requests. Not that I'm your boss giving you work to do or anything like that. 😄

[S0urceror]

Hi @Konamiman, no problem to perform the things you asked. I thought I already did that way back in April and resubmitted. Sorry for the delay. Thanks for chasing this. Will try to do this later this week.

[S0urceror]

Hi @Konamiman, made the requested changes.

But somehow I can't figure out .gitignore. I copy & pasted your version of this file into my project and resubmitted and somehow Github says it's still not correct.

Since I'm now using your .gitignore all DS_Store are appearing again in my GitHub which is a pain that Mac users have to endure I'm afraid.

[S0urceror]

Description According to the Nextor Driver Development Guide a device driver's DEV_RW call should return it's error code via register A.

Problem If a driver sets the error code via a LD A, nn call the flags register is not set. Upon return of the DEV_RW function we need to test the value of A. A value of non-zero means and error. Zero means all okay.

Solution Added AND A and JR Z instructions to handle this.

To test on old version of Nextor Create a device driver, return zero via LD A, 0 and return, depending on the state of the flags an error is triggered while there isn't one.

[Konamiman]

Hi @S0urceror. I have fixed the .gitignore file myself using the GitHub web editor.

If you want these .DS_Store files to be ignored by your local git environment without having to add them to .gitignore, you can do so by adding them (as a .DS_Store/ line) in the .git/info/exclude file, see https://stackoverflow.com/questions/1753070/how-do-i-configure-git-to-ignore-some-files-locally

Now, could you please add an additional commit that:

1. Removes all the existing .DS_Store directories that you pushed.
2. Restores the source/command/command/command2.com file that you deleted. I agree that this file si not needed, but this pull request is not the place to delete it.

I tried to do these changes myself by cloning your branch locally, but I don't have the permissions to push any changes to your repository.

[Konamiman]

Hey @S0urceror, on a second thought, this thing is actually somewhat more complex, since there's a difference in how errors are reported by device-based drivers vs drive-based drivers:

• In device-based drivers, DEV_RW indeed returns an error code in A and that's it, with a value of 0 meaning no error.
• But in drive-based drivers (and this is compatible with MSX-DOS drivers), the carry flag must be set when returning an error from DSKIO, because 0 is an actual valid error code! (it means "Write protected disk").

So upon return of either DEV_RW or DSKIO, the Nextor code should check the carry flag or not, depending on the type of driver (while the relevant code is running, the KSLOT variable temporarily holds 0 for drive-based drivers, and 1 for device-based drivers). The equivalent code for normal mode (in val.mac) is doing that already.

I can implement the required change myself if you want. For that we have two options: either you give me write permissions on your branch, or I close your pull request and then create a new one from scratch. Which one do you prefer?

[S0urceror]

Hi @Konamiman, sounds indeed this is a bit more convoluted. I think it is best you make the change. Please close my original pull request. I think this is the cleanest way to do this.

I did however come up with the following code that might do what we want. But I leave it to you to come up with a better solution.

    ld  ix,DV_DSKIO##   ;Or DEV_RW (they are at the same address)
    ex  af,af'
    ld  a,DV_BANK##
    call    CALBNK##
    push af ; preserve A and flags
    ld  a,(KSLOT##)
    and a
    jr z, DIO_RD_DRIVE_BASED
    ; device based
    pop af ; restore error code
    and a
    jr z,DIO_RD_OK
    jr DIO_RD_ERR
DIO_RD_DRIVE_BASED:
    pop af ; restore error code and flags
    jr nc, DIO_RD_OK
DIO_RD_ERR:
    pop hl  ;On disk error, just return
    pop de
    pop bc
    pop bc  ;Not POP AF, to preserve error info
    jp  CONV_ERR
DIO_RD_OK: