Skip to content

fix null table pointer issue#2

Open
tigerzhang wants to merge 1 commit into
yugabyte:2.9.0-ybfrom
tigerzhang:2.9.0-yb
Open

fix null table pointer issue#2
tigerzhang wants to merge 1 commit into
yugabyte:2.9.0-ybfrom
tigerzhang:2.9.0-yb

Conversation

@tigerzhang

@tigerzhang tigerzhang commented May 9, 2019

Copy link
Copy Markdown

There is a chance meta table is uninitialized. A segment fault will occur.

@kmuthukk

Copy link
Copy Markdown

Thanks @tigerzhang for the pull request submission and contribution.

Would you mind completing this CLA (contributor license agreement) form https://docs.google.com/forms/d/11hn-vBGhOZRunclC3NKmSX1cvQVrU--r0ldDLqasRIo/edit.

@tigerzhang

Copy link
Copy Markdown
Author

signed

@kmuthukk

kmuthukk commented May 26, 2019

Copy link
Copy Markdown

Thx @tigerzhang.

@OlegLoginov - could you please review the pull request?

Comment thread src/metadata.cpp

KeyspaceMetadata* keyspace = NULL;
TableMetadata::Ptr table;
TableMetadata::Ptr table(NULL);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

No need to init it by NULL due to default constructor of the object:
explicit SharedRefPtr(T* ptr = NULL)
So, please undo the change.

Comment thread src/metadata.cpp
}

table->add_index(IndexMetadata::from_row(index_name, buffer, row));
if (table) {

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Agree.
(Just for rare case when 'temp_table_name' is empty string.)

Comment thread src/metadata.cpp
@@ -2434,7 +2434,9 @@ void Metadata::InternalData::update_indexes(int protocol_version, const VersionN
table->clear_indexes();

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

The main problem is here. If the table is not found the 'table_name' must be reset!
So, the line
if (!table) continue;
must be changed into:
if (!table) { table_name.clear(); continue; }

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants