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

Use wrapping behavior on localFile seek position #3779

Merged
merged 1 commit into from
Jun 19, 2024
Merged

Use wrapping behavior on localFile seek position #3779

merged 1 commit into from
Jun 19, 2024

Conversation

weirddan455
Copy link
Collaborator

Description

Tested on MS-DOS 6.22 with some ugly looking assembly code I wrote. The return value from seek wraps. If you're at position 10 and you seek backwards by 20, it returns ~4 billion.

Related issues

Fixes #3778

Manual testing

Implementing this behavior fixes the WinG installer. Blackthorne continues to work.

Checklist

Please tick the items as you have addressed them. Don't remove items; leave the ones that are not applicable unchecked.

I have:

  • followed the project's contributing guidelines and code of conduct.
  • performed a self-review of my code.
  • commented on the particularly hard-to-understand areas of my code.
  • split my work into well-defined, bisectable commits, and I named my commits well.
  • applied the appropriate labels (bug, enhancement, refactoring, documentation, etc.)
  • checked that all my commits can be built.
  • confirmed that my code does not cause performance regressions (e.g., by running the Quake benchmark).
  • added unit tests where applicable to prove the correctness of my code and to avoid future regressions.
  • made corresponding changes to the documentation or the website according to the documentation guidelines.
  • locally verified my website or documentation changes.

Tested on MS-DOS 6.22.
Rather than throwing an error if we would seek before the end of the file,
just return 4 billion and make janky DOS code happy.
@weirddan455 weirddan455 added regression We broke something 😊 Windows 3.x Issues related to Windows 3.x compatibility labels Jun 19, 2024
@weirddan455 weirddan455 self-assigned this Jun 19, 2024
Copy link
Member

@johnnovak johnnovak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice edge case fix @weirddan455. See, this is where using the integrated debugger would've come in handy 😏

@johnnovak johnnovak added the DOS Issues related to DOS integration or DOS commands label Jun 19, 2024
@weirddan455 weirddan455 merged commit bacafaf into main Jun 19, 2024
32 checks passed
@weirddan455
Copy link
Collaborator Author

Nice edge case fix @weirddan455. See, this is where using the integrated debugger would've come in handy 😏

I don't know that the debugger would have helped. I knew what Dosbox was doing. I needed to confirm what MS-DOS was doing so I had to write a DOS program. I used assembly because NASM can output a .COM file and setting up a DOS C compiler is kind of a pain.

@johnnovak
Copy link
Member

I don't know that the debugger would have helped

You could have used your DOS program and inspect the register values in the debugger after a BIOS call, so you could have spared the effort of writing the number to string conversion routine, that's whst I meant.

@weirddan455
Copy link
Collaborator Author

You could have used your DOS program and inspect the register values in the debugger after a BIOS call, so you could have spared the effort of writing the number to string conversion routine, that's whst I meant.

Well I was trying to test in MS-DOS 6.22 in a VM, not inside Dosbox.

@weirddan455 weirddan455 deleted the wd/seek branch June 19, 2024 23:14
@johnnovak
Copy link
Member

johnnovak commented Jun 20, 2024

Well I was trying to test in MS-DOS 6.22 in a VM, not inside Dosbox.

Not sure if you're aware @weirddan455 that you can also run real MS-DOS 6.22 in DOSBox if you mount a FAT HDD partition image. I kinda assumed you did that, that's why I brought up the DOSBox debugger 😄

You can also do freaky stuff like use a dump of a real VGA BIOS instead of our VGA BIOS emulation. I tried it with a few VGA BIOS dumps and it works 😆

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
DOS Issues related to DOS integration or DOS commands regression We broke something 😊 Windows 3.x Issues related to Windows 3.x compatibility
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Repeating seek failures in WINDOWS/SYSTEM/DVA.386 when reinstalling WinG
2 participants