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
CLOOP envelopes is wrong in IStatus [CORE6446] #6679
Comments
Modified by: @AlexPeshkoffassignee: Alexander Peshkov [ alexpeshkoff ] |
Commented by: @AlexPeshkoff Added attribute onError to interface's method in CLOOP. |
Modified by: @AlexPeshkoffstatus: Open [ 1 ] => Resolved [ 5 ] resolution: Fixed [ 1 ] Fix Version: 4.0.0 [ 10931 ] Fix Version: 3.0.8 [ 10960 ] |
Commented by: @asfernandes What is an example of getErrors() / getWarnings() throwing? |
Commented by: @aafemt class MyStatus: public Firebird::IStatusImpl<Status, MyStatusType> |
Commented by: @AlexPeshkoff I attach artificial test, used for checking feature. |
Modified by: @AlexPeshkoffAttachment: CORE6446.artificial_test.cpp [ 13560 ] |
Commented by: @asfernandes IMO this is very artificial and we are talking about really programming errors. (likewise, status could not protect about segfaults). It's very easy to have potential throwable getErrors() / getWarnings() methods to catch exceptions themselves and return the failure status vector. |
Commented by: @aafemt If user's getErrors/getWarnings implementations are supposed never throw then catch() in the envelopes is pointless and must be removed. If the catch() is there then throwing is expected and must be handled properly. That's what this ticket is about. |
Commented by: @asfernandes The envelop is generated in all methods. It's certainly very easy for a developer to be a bit more careful when writing an implementation that is supposed to just deal with exceptions. So far I had not see in Firebird code the problem, so it was certainly easy to make the thing work correctly. Not a big things for me to continue discussing, it's just a thing that "fixes" no relevant problem. How will the envelop deal with wrong getErrors() returning invalid pointers or pointers not adhering to status vector semantics? Will be the next pass be adding IStatus argument to IStatus::getErrors/getWarnings? |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Resolved [ 5 ] QA Status: No test => Cannot be tested |
Modified by: @pavel-zotovstatus: Resolved [ 5 ] => Closed [ 6 ] |
Commented by: @AlexPeshkoff If we request from "a developer to be a bit more careful when writing an implementation that is supposed to just deal with exceptions" what for are envelopes at all? |
Submitted by: @aafemt
Attachments:
CORE-6446.artificial_test.cpp
If CLOOP envelope in IStatusImpl::getErrors()/getWarnings() catch an exception it returns null pointer and crash the engine.
It should return pointer to a static array the same as BaseStatusWrapper::catchException().
Commits: 52ea547 e039e01
The text was updated successfully, but these errors were encountered: