-
Notifications
You must be signed in to change notification settings - Fork 327
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
out-of-memory condition is handled widely inconsistently #1008
Comments
I do not remember ever getting a bugreport for this kind of thing. I would not spend effort on it currently , only recommend defensive programming in future code. |
That just proves my point :). Having NULL checks for maybe 30% of all those (tiny) allocations is pointless because a) they never fail and b) if one would, the user still has a 70% chance of crashing his process. I'll not venture into any effort to make this part of the code base consistent, though. Thanks for your input. |
To be able to use the array macros in PTP and GP error "contexts", this makes translate_ptp_result() tolerate both "types" of errors. I added a comment regarding the use of the GP_ERROR_NO_MEMORY use. This is related to gphoto#1008.
To be able to use the array macros in PTP and GP error "contexts", this makes translate_ptp_result() tolerate both "types" of errors. I added a comment regarding the use of the GP_ERROR_NO_MEMORY use. This is related to #1008.
There are currently almost 1000 calls to memory allocating functions (
malloc
,calloc
,realloc
,strdup
) all over the code base. There are 3 inconsistent levels of dealing with a failed allocation:NULL
and return an error (mostlyGP_ERROR_NO_MEMORY
orPTP_RC_GeneralError
)Option 1) occurs the most by far, option 3) only very seldomly.
I would argue that option 3) is definitively overkill and merely introduces noise in the code base. Realistically, I'd argue that option 2) is also virtually useless in real-world scenarios. If your process ran out of memory, it will shut down either way, pretty soon. To my knowledge, on Linux, it might very well be killed by the OOM killer before you ever get a failed malloc.
My suggestion would be to either completely do away with verbose NULL checks for failed allocations (only use option 1), which will then result in SEGFAULTS. Or come up with something like xalloc that puts out some kind of "out of memory" error message and then aborts the program, which obviously provides more information than simply SEGFAULting.
Leaving the status quo is of course also an option... ;)
The text was updated successfully, but these errors were encountered: