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

rgw: Initialization of epoch,len #17722

Merged
merged 1 commit into from Oct 23, 2017

Conversation

Projects
None yet
4 participants
@amitkumar50
Copy link
Contributor

commented Sep 14, 2017

Fixes the coverity issues:

** 1402628 Uninitialized scalar field

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

** 1409841 Uninitialized scalar field

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

** 1416594 Uninitialized scalar field

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

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

@@ -177,7 +177,7 @@ namespace rgw {

class RGWFileHandle : public cohort::lru::Object
{
struct rgw_file_handle fh;
struct rgw_file_handle fh {};

This comment has been minimized.

Copy link
@joscollin

joscollin Sep 15, 2017

Member

Nit: remove the unnecessary space before {

@amitkumar50 amitkumar50 force-pushed the amitkumar50:cov-rgw-6 branch from ba7b7c9 to 3fed72d Sep 16, 2017

@amitkumar50

This comment has been minimized.

Copy link
Contributor Author

commented Sep 16, 2017

@joscollin Done Thanks

@joscollin joscollin added the needs-qa label Sep 18, 2017

@joscollin

This comment has been minimized.

Copy link
Member

commented Sep 18, 2017

retest this please

@joscollin

This comment has been minimized.

Copy link
Member

commented Sep 25, 2017

retest this please

@@ -177,7 +177,7 @@ namespace rgw {

class RGWFileHandle : public cohort::lru::Object
{
struct rgw_file_handle fh;
struct rgw_file_handle fh{};

This comment has been minimized.

Copy link
@mattbenjamin

mattbenjamin Sep 25, 2017

Contributor

I don't see a reason to introduce this syntax, which is not part of the prevailing style. If you believe fh can ever be uninitialized, please initialize it in the applicable constructors. Note that objects of this type are required to be created by a factory method, which also carries weight here.

This comment has been minimized.

Copy link
@joscollin

joscollin Sep 25, 2017

Member

@mattbenjamin What you suggested is my preferred method too - initialize those variable in the constructors. But I saw that many of these structures (initialized with {}) are having structure / enum variables inside. So those variables should be default initialized in their own constructors too. So that is the reason I thought {} would be simple.

struct rgw_file_handle
{
  struct rgw_fh_hk fh_hk;
  void *fh_private;
  enum rgw_fh_type fh_type;
};

This comment has been minimized.

Copy link
@mattbenjamin

mattbenjamin Sep 25, 2017

Contributor

right, it's a C structure; I think its fine to initalize it in the RGWFileHandle ctors

This comment has been minimized.

Copy link
@joscollin

joscollin Sep 26, 2017

Member

@mattbenjamin Agreed. fh_hk initialization is missing in RGWFileHandle(RGWLibFS* _fs) {}. The other places seems to be okay.

This comment has been minimized.

Copy link
@mattbenjamin

mattbenjamin Sep 27, 2017

Contributor

@amitkumar50 can you please update?

This comment has been minimized.

Copy link
@amitkumar50

amitkumar50 Sep 28, 2017

Author Contributor

@mattbenjamin @joscollin Working on it. Thanks For comment

@mattbenjamin

This comment has been minimized.

Copy link
Contributor

commented Sep 27, 2017

@amitkumar50 can you please update?

@amitkumar50 amitkumar50 force-pushed the amitkumar50:cov-rgw-6 branch from 3fed72d to d71d304 Sep 28, 2017

@amitkumar50

This comment has been minimized.

Copy link
Contributor Author

commented Sep 28, 2017

@mattbenjamin @joscollin Done Changes Thanks

@@ -262,7 +262,7 @@ namespace rgw {

private:
RGWFileHandle(RGWLibFS* _fs)
: fs(_fs), bucket(nullptr), parent(nullptr), variant_type{directory()},
: fh{}, fs(_fs), bucket(nullptr), parent(nullptr), variant_type{directory()},

This comment has been minimized.

Copy link
@joscollin

joscollin Sep 28, 2017

Member

As fh.fh_type and fh.fh_private are already initialized here down below, the remaining member is only fh_hk. So do:

fh.fh_hk.bucket = 0;
fh.fh_hk.object = 0;

the similar way inside the constructor, For being specific and to avoid {}.

@@ -301,7 +301,7 @@ namespace rgw {
public:
RGWFileHandle(RGWLibFS* _fs, RGWFileHandle* _parent,
const fh_key& _fhk, std::string& _name, uint32_t _flags)
: fs(_fs), bucket(nullptr), parent(_parent), name(std::move(_name)),
: fh{}, fs(_fs), bucket(nullptr), parent(_parent), name(std::move(_name)),

This comment has been minimized.

Copy link
@joscollin

joscollin Sep 28, 2017

Member

Drop this, as fh.fh_type, fh.fh_hk and fh.fh_private are getting initialized below in the same constructor. I don't find any missing branches in this constructor, where they are not getting initialized.

@amitkumar50 amitkumar50 force-pushed the amitkumar50:cov-rgw-6 branch from d71d304 to 17df8c9 Oct 19, 2017

@amitkumar50

This comment has been minimized.

Copy link
Contributor Author

commented Oct 19, 2017

@jcollions Done changes

rgw: Initialization of epoch,len
Fixes the coverity issues:

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

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

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

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

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2017

@yuriw

This comment has been minimized.

Copy link
Contributor

commented Oct 23, 2017

ready for merge if all comments are done

@mattbenjamin mattbenjamin merged commit 6bef8e3 into ceph:master Oct 23, 2017

5 checks passed

Docs: build check OK - docs built
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details
make check make check succeeded
Details
make check (arm64) make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.