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
SQZ_apply_differences [CORE3148] #3525
Comments
Modified by: @AlexPeshkoffassignee: Alexander Peshkov [ alexpeshkoff ] |
Commented by: Ann Harrison (awharrison) That is about the most commonly executed code in Firebird - I suspect your |
Commented by: @AlexPeshkoff Ann, I understand your worries about performance. And I do not believe in compilers that produce same efficiency code with added conditional check inside the loop:-) Unfortunately without suggested by Dmitry test we have real possibility of segfault when working with corrupted database. Imagine end of differences exactly at the end of OS-allocated memory segment. If we read after the end, we get segfault. |
Commented by: @AlexPeshkoff As was expected, this fix makes test, specially oriented on massive differences applies, work slower - 0.17%. Taking into an account that deviation is series of test is 5 times greater and this is artificial test, I suppose patch can be applied. |
Commented by: @AlexPeshkoff Expected performace drawback is so small taht I'ev found it's possible to use original solution |
Modified by: @AlexPeshkoffstatus: Open [ 1 ] => Resolved [ 5 ] resolution: Fixed [ 1 ] Fix Version: 2.1.4 [ 10361 ] Fix Version: 2.5.1 [ 10333 ] Fix Version: 3.0 Alpha 1 [ 10331 ] Fix Version: 2.0.7 [ 10390 ] |
Modified by: @pcisarstatus: Resolved [ 5 ] => Closed [ 6 ] |
Modified by: @pavel-zotovQA Status: No test |
Modified by: @pavel-zotovstatus: Closed [ 6 ] => Closed [ 6 ] QA Status: No test => Cannot be tested |
Submitted by: @ibprovider
Current code:
------------------ [BEGIN]
while (differences < end && p < p_end)
{
const SSHORT l = *differences++;
if (l > 0)
{
if (p + l > p_end)
{
BUGCHECK(177); /* msg 177 applied differences will not fit in record */
}
memcpy(p, differences, l); // <--------- CAN READ AFTER LAST ELEMENT OF differences
------------------ [END]
I offer to rewrite this function
------------------ [BEGIN]
USHORT SQZ_apply_differences(Record* const record, const SCHAR* differences, const SCHAR* const end)
{
/**************************************
*
* S Q Z _ a p p l y _ d i f f e r e n c e s
*
**************************************
*
* Functional description
* Apply a differences (delta) to a record. Return the length.
*
**************************************/
typedef USHORT result_type;
}//SQZ_apply_differences
------------------ [END]
Commits: 19e53d3 ffd0aaa 15132bc 88e0503
The text was updated successfully, but these errors were encountered: