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.

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

cls/rgw: Initialization of uninitialized members #16932

Merged
merged 1 commit into from Dec 29, 2017

Conversation

amitkumar50
Copy link

Fixes the coverity issues:

** 1396131 Uninitialized scalar field

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

** 1396125 Uninitialized scalar field

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

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

@@ -1045,7 +1045,7 @@ WRITE_CLASS_ENCODER(cls_rgw_gc_obj_info)

struct cls_rgw_lc_obj_head
{
time_t start_date;
time_t start_date = 0;
Copy link
Member

Choose a reason for hiding this comment

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

It depends on what date should be the start_date. If it is the system's current date, then initialize with time(0);. Please verify it.

Copy link
Author

Choose a reason for hiding this comment

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

@joscollin Yes It looks.. It would be system's current time.
Why?
Since void encode(bufferlist& bl) calls ::encode(t, bl); with start_time, I believe encode would not start on 0, It would start on Current time.

But It would be good to take 2nd opinion from any other reviewer as well...

Copy link
Member

Choose a reason for hiding this comment

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

@tchaikov Could you please take a look ?

Copy link
Author

Choose a reason for hiding this comment

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

@tchaikov @joscollin Done Changes Thanks.

@amitkumar50
Copy link
Author

@joscollin Done changes..

@tchaikov tchaikov removed their request for review August 16, 2017 11:24
@joscollin
Copy link
Member

Jenkins retest this please

@liewegas liewegas requested a review from tchaikov August 29, 2017 22:21
@@ -1045,7 +1045,7 @@ WRITE_CLASS_ENCODER(cls_rgw_gc_obj_info)

struct cls_rgw_lc_obj_head
{
time_t start_date;
time_t start_date = time(0);
Copy link
Contributor

Choose a reason for hiding this comment

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

no point to get current time here. please just use 0.

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

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

Signed-off-by: Amit Kumar amitkuma@redhat.com
@tchaikov tchaikov requested a review from dang August 31, 2017 08:21
@tchaikov
Copy link
Contributor

@dang and @chenji-kael , could you help review this change? i failed to find the place where we set the cls_rgw_lc_obj_head::start_date

Copy link
Contributor

@tchaikov tchaikov left a comment

Choose a reason for hiding this comment

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

please hold until we understand where/when start_date should be set.

@amitkumar50
Copy link
Author

@dang @chenji-kael Kindly have some time to look this PR..
Great Thanks

@theanalyst
Copy link
Member

start_date is set in https://github.com/ceph/ceph/blob/master/src/rgw/rgw_lc.cc#L197 and see https://github.com/ceph/ceph/blob/master/src/rgw/rgw_lc.cc#L642, But we currently have LC tests disabled in master due to frequent timing issues, #17020 is trying to fix that

@amitkumar50
Copy link
Author

@theanalyst So What needed to be done now?

@tchaikov tchaikov dismissed their stale review October 19, 2017 07:46

thanks @theanalyst, but i am not qualified to approve this PR.

@liewegas liewegas merged commit 874ac9c into ceph:master Dec 29, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants