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

int13h Read and write don't check for errors #6

Open
joshudson opened this issue Nov 26, 2023 · 1 comment · May be fixed by #7
Open

int13h Read and write don't check for errors #6

joshudson opened this issue Nov 26, 2023 · 1 comment · May be fixed by #7

Comments

@joshudson
Copy link

joshudson commented Nov 26, 2023

I was looking into adding LBA bios extensions (I have plans...) and I found this issue.

extended_read_disk always writes its return value to AL (not AX), but the invocation does:

	mov	ah, 0
	cpu	186
	shl	ax, 9
	extended_read_disk
	shr	ax, 9
	cpu	8086
	mov	ah, 0x02	; Put read code back

	cmp	al, 0
	je	rd_error

throwing AL away after extended_read_disk. Here I found disk read errors are thrown away. I was scratching my head on how this worked because extended_read_disk clearly sets AL to 0 on a successful read, but I finally figured out that AL is thrown away by the SHR instruction; thus the error code is never checked.

It's almost like the code wants the caller to do a ~read like the ~lseek, but even so that does nothing until this code is fixed. The following assembly change should fix the errors not being checked but will immediately break until the C code is fixed:

	sub  al, 1
	adc  ah, 0        ; Preserve CF through the following SHR
	shr	ax, 9
	cpu	8086
	mov	ah, 0x02	; Put read code back
	
	jc	rd_error

Checking lseek is useless anyway: lseek will happily seek out of bounds. Only checking read or write matters. Thus, the error code returned is always wrong. So rd_error and wr_error need work.

@joshudson joshudson changed the title Dodgy code in read int13h Read and write don't check for errors Nov 26, 2023
@joshudson
Copy link
Author

I do have this fixed on my home PC; not pushing anything until I get int 13h ah=42h debugged.

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.

1 participant