Add a bunch of sanity checks

[Megamouse]

Some of these changes are meant to fix a couple of warnings while reading the code.
I also noticed that there were almost no sanity checks in the code, which may lead to lots of segfaults in user code.

• Win: Do not call CloseHandle in hid_open_path if device_handle is null
• Win: return errors if malloc calls fail
• Add missing NULL checks to many parameters

[Youw]

functions like this are used only internally
is it even possible accidentally pass an invalid dev here? I'm pretty sure if the user passes an invalid or NULL dev into any of the hid_* functions - there woud be an undefined behavior (crash) way before we get here in most places...

[Megamouse]

I don't know, I didn't discriminate when I added these.
In my opinion there shouldn't be any function without sanity checks.
It's not unthinkable that this was possible before I added the other checks after all.

[Youw]

In my opinion there shouldn't be any function without sanity checks.

That is actually debatable. Most of the checks in this PR are actually do more harm then help.
See my other comment below.

[Megamouse]

I think I made all the necessary changes.

[Youw]

early return here as well - it doesn't make sense to call this function with NULL cur_dev at all

[Youw]

all public API (specifically functions that accept hid_device * as first argument):

• do not make sens to call with non-valid dev
• should register an error string in case if anything goes wrong, so the user could use hid_error to check what went wrong in case if -1 is returned

I do not support having this checks for all public API functions (marked with HID_API_CALL/HID_API_EXPORT_CALL).
We always assume that the dev should be valid, and that is up to the user to make sure it is. (It is fairly easy to implement in other languages that wrap this library w/o any additional runtime checks by using e.g. class invariant).
The only exception is hid_close function - similar to free it should gracefully accept NULL-device.

All other functions the accept hid_device * as an argument called from public API function should use the same assumption, that the dev is valid and doesn't require extra checks.

[Megamouse]

Given that most of these functions already have sanity checks including hid_error for other parameters, I kept the checks and added report_global_error calls.

[Youw]

this is 100% not a supported scenario and should not be handled explicitly in such way
pieces like this create too much "noise/distraction" in the code that actually makes the overall implementation worse than better

please revert

[Megamouse]

So you'd rather have a hard crash on this than an error that the user can actually catch...

[Youw]

Let me rephrase that (and that is not only my opinion, but rather shared experience of many developers/projects I've seen):
if I as a user messed up some implementation - I'd rather get a crash immediately (and have a chance to fix it immediatelly), that a silent error (often hidden due to lack of error-handling code) which is missed and then the real problem (the one that affects some business scenarios on your application) hits later when something slightly changed in a different part of my application.

[Megamouse]

I can remove it then. Although I would probably have to double check if there isn't an unguarded call to this.
I can make a separate PR with the obvious changes that don't need discussion (like the malloc ptr checks).

[Youw]

I can make a separate PR with the obvious changes that don't need discussion (like the malloc ptr checks).

Great idea, lets start with that.

[Youw]

NOTE: in #698 I've added some of those

[Megamouse]

If you tell me what to keep and what to remove I can adjust this soon

[Youw]

I guess I had too many comments/questions to this PR.
Instead I prepared #727 based on this, with all my comments applied.