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

fault.cpp #1422

Open
2 of 8 tasks
AJenbo opened this issue Aug 29, 2019 · 6 comments
Open
2 of 8 tasks

fault.cpp #1422

AJenbo opened this issue Aug 29, 2019 · 6 comments
Milestone

Comments

@AJenbo
Copy link
Member

AJenbo commented Aug 29, 2019

  • fault_init_filter
  • TopLevelExceptionFilter
  • fault_unknown_module
  • fault_call_stack
  • fault_get_error_type
  • fault_reset_filter
  • log_create
  • fault_set_filter [hellfire] fault_set_filter #1989
@AJenbo AJenbo added this to the Hellfire milestone Aug 29, 2019
@AJenbo
Copy link
Member Author

AJenbo commented Oct 18, 2020

It's a mess an no one cares

@AJenbo AJenbo closed this as completed Oct 18, 2020
@ChaosMarc
Copy link
Contributor

@AJenbo Why did you close this issue? Are all functions of this file binary exact?

@qndel
Copy link
Member

qndel commented Oct 18, 2020

no, we just don't care

@AJenbo
Copy link
Member Author

AJenbo commented Oct 18, 2020

@ChaosMarc we know the functions in this file well enough to say that they have nothing to do with game pay. They are simply and older form of Diablo's crash handeling and very Windows specific. Diablo 1.09 has better crash handling. Between the two versions crash handling was rewritten so having both versions becomes a bit messy.
As my focus is on cross platform the Windows specific code also doesn't have much interest, and we have even better tools at this time. No one else on the current team appears interested in spending the time working on this file and so we removed it from the todo list. If anyone else wants to do it for copleatness sake they are of cause welcome to do so.

@ChaosMarc
Copy link
Contributor

@AJenbo thank you very much for the explanation. I can definitely understand the reasoning. It's frustrating to work on code for which there is a better version at Handy already just for completions sake. So if I understood correctly you will use the 1.09 error handling for hellfire as well, right? This is probably an improvement but is it fully compatible? But won't this affected other files' bin exactness? But hey, as I am an ocd level completionist, maybe I will look into it myself :D

@AJenbo
Copy link
Member Author

AJenbo commented Oct 19, 2020

Normally one function won't affect the bin exactness of another as long as the signature is the same. There is however a chance that the different size could affect alignment, but that is also true for differing number of enums, headers and other things that aren't suppose to affect the output but still causes some strange behavior in VC 5/6. If that happens i guess we can just add some more enums with would help code quality any way :D

You are most welcome to work on it, any help is a appreciated and I would be happy to help you get started.

The error handling from 1.09 will work perfectly fine in hellfire, it all gets loaded in to the windows api at boot up and doesn't have any special knowledge about the rest of the system or vise versa.

The only call to the code in fault.cpp is in WinMain which we have already gotten bin exact, meaning that we have figured out the correct signature:

lpTopLevelExceptionFilter = SetUnhandledExceptionFilter(diablo_TopLevelExceptionFilter);

@AJenbo AJenbo added the low priority This should be worked on later label Dec 11, 2020
@AJenbo AJenbo reopened this Dec 11, 2020
@AJenbo AJenbo removed the low priority This should be worked on later label Jan 16, 2021
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

No branches or pull requests

3 participants