Skip to content
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.

Sign up for GitHub

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

Jump to bottom

rgw: Initializes uninitialized members #16855

Merged
merged 1 commit into from Sep 3, 2017
Merged

rgw: Initializes uninitialized members #16855

merged 1 commit into from Sep 3, 2017

Conversation

amitkumar50
Copy link

Fixes the coverity issues:

** 1352181 Uninitialized scalar field
2. uninit_member: Non-static class member field fh_hk.bucket is
not initialized in this constructor nor in any functions that it calls.
CID 1352181 (#1 of 1): Uninitialized scalar field (UNINIT_CTOR)
4. uninit_member: Non-static class member field fh_hk.object is
not initialized in this constructor nor in any functions that it calls.

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

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

Signed-off-by: Amit Kumar amitkuma@redhat.com

@joscollin joscollin self-requested a review August 7, 2017 06:17
joscollin
joscollin requested changes Aug 7, 2017
View reviewed changes
src/rgw/rgw_file.h Outdated
@@ -91,7 +91,7 @@ namespace rgw {
*/
struct fh_key
{
rgw_fh_hk fh_hk;
rgw_fh_hk fh_hk {0};
Copy link
Member

Choose a reason for hiding this comment

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

I don't see a constructor in rgw_fh_hk that accepts an integer to do this change.

Otherwise, it should be like this: http://en.cppreference.com/w/cpp/language/value_initialization

Copy link
Author

Choose a reason for hiding this comment

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

@joscollin Thanks for comment.
I believe something as:
rgw_fh_hk fh_hk {};

Copy link
Author

Choose a reason for hiding this comment

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

@joscollin Can you please respond

Copy link
Contributor

Choose a reason for hiding this comment

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

i think rgw_fh_hk fh_hk {}; is good here

Copy link
Member

Choose a reason for hiding this comment

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

@cbodley Yes, I meant the same. Thanks.

src/rgw/rgw_common.h Outdated
@@ -1197,7 +1197,7 @@ struct RGWBucketInfo
bool has_website;
RGWBucketWebsiteConf website_conf;

RGWBucketIndexType index_type;
RGWBucketIndexType index_type = RGWBIType_Indexless;

Copy link
Member

Choose a reason for hiding this comment

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

Why RGWBIType_Indexless ? Any justification ?

Copy link
Author

Choose a reason for hiding this comment

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

@joscollin Here is what i interpreted
Inside decode function
void decode(bufferlist::iterator& bl){
if (struct_v >= 15) {
uint32_t it;
::decode(it, bl);
index_type = (RGWBucketIndexType)it; //This would be decoded. We can consider would be valid index not Indexless
} else {
index_type = RGWBIType_Normal; //says normal Index
}
}

So I feel Indexless is bad case here. But your opinion would be valuable.

Copy link
Author

Choose a reason for hiding this comment

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

@joscollin Can you please provide feedback?

Copy link
Author

Choose a reason for hiding this comment

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

@joscollin Can you please respond??

Copy link
Contributor

Choose a reason for hiding this comment

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

RGWBIType_Normal is probably a better default

Copy link
Member

Choose a reason for hiding this comment

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

@cbodley I also think RGWBIType_Normal is the default value, considering the above decode() logic.

amitkumar50
amitkumar50 commented Aug 14, 2017
View reviewed changes
Copy link
Author

@amitkumar50 amitkumar50 left a comment

Choose a reason for hiding this comment

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

@joscollin can you please comment on my comments..

src/rgw/rgw_common.h Outdated
@@ -1197,7 +1197,7 @@ struct RGWBucketInfo
bool has_website;
RGWBucketWebsiteConf website_conf;

RGWBucketIndexType index_type;
RGWBucketIndexType index_type = RGWBIType_Indexless;

Copy link
Author

Choose a reason for hiding this comment

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

@joscollin Here is what i interpreted
Inside decode function
void decode(bufferlist::iterator& bl){
if (struct_v >= 15) {
uint32_t it;
::decode(it, bl);
index_type = (RGWBucketIndexType)it; //This would be decoded. We can consider would be valid index not Indexless
} else {
index_type = RGWBIType_Normal; //says normal Index
}
}

So I feel Indexless is bad case here. But your opinion would be valuable.

src/rgw/rgw_file.h Outdated
@@ -91,7 +91,7 @@ namespace rgw {
*/
struct fh_key
{
rgw_fh_hk fh_hk;
rgw_fh_hk fh_hk {0};
Copy link
Author

Choose a reason for hiding this comment

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

@joscollin Thanks for comment.
I believe something as:
rgw_fh_hk fh_hk {};

joscollin
joscollin requested changes Aug 31, 2017
View reviewed changes
src/rgw/rgw_common.h Outdated
@@ -1197,7 +1197,7 @@ struct RGWBucketInfo
bool has_website;
RGWBucketWebsiteConf website_conf;

RGWBucketIndexType index_type;
RGWBucketIndexType index_type = RGWBIType_Indexless;

Copy link
Member

Choose a reason for hiding this comment

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

@cbodley I also think RGWBIType_Normal is the default value, considering the above decode() logic.

src/rgw/rgw_file.h Outdated
@@ -91,7 +91,7 @@ namespace rgw {
*/
struct fh_key
{
rgw_fh_hk fh_hk;
rgw_fh_hk fh_hk {0};
Copy link
Member

Choose a reason for hiding this comment

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

@cbodley Yes, I meant the same. Thanks.

@amitkumar50
Copy link
Author

@joscollin Done changes. Thanks

joscollin
joscollin approved these changes Aug 31, 2017
View reviewed changes
Copy link
Member

@joscollin joscollin left a comment

Choose a reason for hiding this comment

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

LGTM

rgw: Initializes uninitialized members of rgw
4bca8f8 
Fixes the coverity issues:

** 1352181 Uninitialized scalar field
2. uninit_member: Non-static class member field fh_hk.bucket is
not initialized in this constructor nor in any functions that it calls.
CID 1352181 (ceph#1 of 1): Uninitialized scalar field (UNINIT_CTOR)
4. uninit_member: Non-static class member field fh_hk.object is
not initialized in this constructor nor in any functions that it calls.

** 1353424 Uninitialized scalar field
CID 1353424 (ceph#1 of 1): Uninitialized scalar field (UNINIT_CTOR)
5. uninit_member: Non-static class member watch_handle is not initialized
 in this constructor nor in any functions that it calls.

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

Signed-off-by: Amit Kumar <amitkuma@redhat.com>
@joscollin
Copy link
Member

Test Results:

@yehudasa yehudasa merged commit f609c0e into ceph:master Sep 3, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@cbodley cbodley cbodley approved these changes

@joscollin joscollin joscollin approved these changes

@mattbenjamin mattbenjamin Awaiting requested review from mattbenjamin

Assignees
No one assigned
Labels
cleanup needs-qa rgw wip-jcollin-testing
Projects
None yet
Milestone
No milestone
4 participants
@amitkumar50 @joscollin @cbodley @yehudasa