Skip to content

Replace useless NilAccessDefect with useful LibraryError#1

Open
theAkito wants to merge 2 commits intokhchen:masterfrom
theAkito:master
Open

Replace useless NilAccessDefect with useful LibraryError#1
theAkito wants to merge 2 commits intokhchen:masterfrom
theAkito:master

Conversation

@theAkito
Copy link

First of all, thank you for yet another great library!

I wanted to contribute at least a bit, by fixing an issue I found when testing this library. 😃

theAkito added 2 commits May 31, 2021 03:26
Raising the `LibraryError` in line 313 shows exactly what library failed to load.
If the `handle` variable assignment excepts, due to a `NilAccessDefect`,
then you first have to debug the flow of your program to find the issue.
Therefore, we catch the exception and raise a meaningful error,
that is telling you exactly why the `LoadLibraryA` proc failed to execute.
@khchen
Copy link
Owner

khchen commented May 31, 2021

Thank you for this PR! My questions:

  1. How do you let LoadLibraryA(cname) raise a NilAccessDefect? bad DLL?
  2. Is there other codes that cause useless exception instead of LibraryError for bad DLL?

@theAkito
Copy link
Author

Thank you for this PR! My questions:

  1. How do you let LoadLibraryA(cname) raise a NilAccessDefect? bad DLL?
  2. Is there other codes that cause useless exception instead of LibraryError for bad DLL?
  1. To be honest, I'm still puzzled. After this Patch, it's telling me that it can't find "KERNEL32.dll" -- even though, this should be impossible, as far as I understand.
  2. Yes, there are other places in the code that could be improved by replacing NilAccessDefect with something meaningful, but I did not know about the others yet, when I opened this PR.

If you think this method for making more useful exceptions is helpful, perhaps it would be even better if all places where a NilAccessDefect could possibly be found would be improved by a more meaningful exception, in the same way as done in this commit (try catch all the possible failures)?

To put all this in context: I was playing around with the openssl example, because I wanted to test if I can embed the library into the executable with memlib. However, I am still struggling to get it to work.

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 this pull request may close these issues.

2 participants