-
Notifications
You must be signed in to change notification settings - Fork 13
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
remove throw from gdaljl_errorhandler #153
Conversation
The throw interrupts control flow on the C side, e.g. suppressing the closing of file descriptors when the error is caught by julia.
This is all that is required to fix tests after JuliaGeo/GDAL.jl#153
This is all that is required to fix tests after JuliaGeo/GDAL.jl#153
Co-authored-by: Martijn Visser <[email protected]>
Oh, before I forget: There is an alternative how errors could be handled. One could have a global const ref to a GDALError that gets passed as user data to the registered error handler function and gets updated inside. This would require registering the gdaljl_errorhandler at load time I think, because the address the ref points to will always be different. But all this sounds bad and should only be considered if it is really necessary. |
Yeah that sounds like a worse option. I'm ok with this, I'll leave it open for a bit to give people a chance to react. |
Do we lose anything losing this error printing? Or things will print anyway from |
If we loose the error printing, we could at least log the failure number and error message. |
We don't lose the error printing with this. The I think the reason I kept both systems in place was because I wasn't sure if the check from the julia side would catch all the errors that the error handler catches. But I don't know any examples of that. Another thing that I can imagine that one ccall sets first one, more informative error, and then another. In this new approach we would no longer see the first, but only the last. Though I haven't seen any examples of this either. I guess generally on CE_Failure or CE_Fatal the ccall would return anyway. |
Makes sense, we can live with just the last error. Im happy with this then. |
The throw interrupts control flow on the C side, e.g. suppressing the closing of file descriptors when the error is caught by julia. This PR removes it, but leaves the error handler in place, so that printing by GDAL is suppressed. Furthermore, maybe_throw now also throws on fatal errors. Locally, all test pass.
ref rafaqz/Rasters.jl#455