Skip to content

options: Fix coverity issues#3106

Closed
pdvian wants to merge 2 commits intofacebook:masterfrom
pdvian:wip-fix-coverity-issues-1396208
Closed

options: Fix coverity issues#3106
pdvian wants to merge 2 commits intofacebook:masterfrom
pdvian:wip-fix-coverity-issues-1396208

Conversation

@pdvian
Copy link
Copy Markdown

@pdvian pdvian commented Nov 1, 2017

Summary:
options/cf_options.cc:
77 memtable_insert_with_hint_prefix_extractor(

CID 1396208 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR)
2. uninit_member: Non-static class member info_log_level is not initialized in this constructor nor in any functions that it calls.

Summary:
options/cf_options.cc:
 77      memtable_insert_with_hint_prefix_extractor(

CID 1396208 (facebook#1 of 1): Uninitialized scalar field (UNINIT_CTOR)
2. uninit_member: Non-static class member info_log_level is not initialized in this constructor nor in any functions that it calls.
 78          cf_options.memtable_insert_with_hint_prefix_extractor.get()) {}

include/rocksdb/advanced_options.h:

AdvancedColumnFamilyOptions::AdvancedColumnFamilyOptions() {
 40  assert(memtable_factory.get() != nullptr);

CID 1405359 (facebook#1 of 1): Uninitialized scalar field (UNINIT_CTOR)
2. uninit_member: Non-static class member max_mem_compaction_level is not initialized in this constructor nor in any functions that it calls.
 41}

As max_mem_compaction_level is not supported anymore, undefine it.
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@pdvian has updated the pull request. View: changes

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Can you instead initialize this option to 0? It is leave here to avoid users hitting compile error.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@yiwu-arbug I have commented out all the references to max_mem_compaction_level variable from struct AdvancedColumnFamilyOptions. Would we still want to use max_mem_compaction_level variable ?

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Let's keep max_mem_compaction_level for now.

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@yiwu-arbug made the changes. There are 2 commits on this PR, but pick 37f163c only. Let me know if you want me to open another PR if things are messy.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

should initialize to db_options.info_log_level

@pdvian pdvian force-pushed the wip-fix-coverity-issues-1396208 branch from 7a15eb5 to 37f163c Compare December 12, 2017 06:27
@facebook-github-bot
Copy link
Copy Markdown
Contributor

@pdvian has updated the pull request.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@sagar0 has imported this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Copy link
Copy Markdown
Contributor

@facebook-github-bot facebook-github-bot left a comment

Choose a reason for hiding this comment

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

@sagar0 is landing this pull request. If you are a Facebook employee, you can view this diff on Phabricator.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants