Skip to content
/ server Public

coverity maria repair parallel#419

Merged
svoj merged 1 commit intoMariaDB:5.5from
grooverdan:5.5-coverity-maria_repair_parallel
Jul 18, 2017
Merged

coverity maria repair parallel#419
svoj merged 1 commit intoMariaDB:5.5from
grooverdan:5.5-coverity-maria_repair_parallel

Conversation

@grooverdan
Copy link
Member

I submit this under the MCA.

Copy link
Contributor

@svoj svoj left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

my_b_clear() should be enough.
bzero() is generally deprecated and we should use memset.

@svoj svoj added this to the 5.5 milestone Jul 6, 2017
@svoj svoj self-assigned this Jul 6, 2017
@svoj
Copy link
Contributor

svoj commented Jul 6, 2017

And then condition must change to if (my_b_inited()) end_io_cache();

@svoj
Copy link
Contributor

svoj commented Jul 11, 2017

@grooverdan we have 5.5 week now. Please update if you want to get this into next 5.5 release.

@grooverdan
Copy link
Member Author

thanks for the heads up.

@grooverdan grooverdan force-pushed the 5.5-coverity-maria_repair_parallel branch from 0c5b3fd to b874542 Compare July 14, 2017 02:15
end_io_call uses uninitialized values from the new_data_cache

As such we the buffer 0 and check this before calling end_io_cache on it.

Thanks Sergey Vojtovich for the review and for this solution.

Found by Coverity (ref 972481).
@grooverdan grooverdan force-pushed the 5.5-coverity-maria_repair_parallel branch from b874542 to 44d74a7 Compare July 14, 2017 03:49
*/
DBUG_PRINT("info", ("is quick repair: %d", (int) rep_quick));
if (!rep_quick)
my_b_clear(&new_data_cache);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Taking into account simplicity of my_b_clear() and my_b_inited() I'd rather do my_b_clear() unconditionally. It saves 2 conditional jumps, which performance wise are worse than single unconditional store.

This is just to voice my opinion, current implementation is generally fine too. No action needed.

@svoj svoj merged commit c9883b7 into MariaDB:5.5 Jul 18, 2017
@svoj
Copy link
Contributor

svoj commented Jul 18, 2017

Thanks!

@grooverdan
Copy link
Member Author

works for me. Thanks.

@grooverdan grooverdan deleted the 5.5-coverity-maria_repair_parallel branch July 18, 2017 06:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

2 participants