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

src/kv/MemDB.cc: the type of the parameter of push_back() does not match the ops's value_type #10455

Merged
merged 1 commit into from Aug 10, 2016

Conversation

wjwithagen
Copy link
Contributor

  • NULL is a C-type usuage, which is allow by GCC.
    But Clang is more strick in checking:

Error looks like:
src/kv/MemDB.cc:228:7r error: no matching member function
for call to 'push_back'
ops.push_back(make_pair(DELETE,

/usr/include/c++/v1/vector:685:36: note: candidate function not viable:
no known conversion from 'pair<[...], pair<[...], nullptr_t>>' to
      'const pair<[...], pair<[...], ceph::buffer::list>>' for 1st argument
    _LIBCPP_INLINE_VISIBILITY void push_back(const_reference __x);
                                   ^

Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>

@tchaikov
Copy link
Contributor

tchaikov commented Jul 27, 2016

lgtm modulo the commit message, it's sort of misleading. the reason why clang fails to build MemDB.cc is not that NULL is a C style thing, it is due to that the type of the parameter of push_back() does not match the ops's value_type.

@chhabaramesh what do you think?

@tchaikov
Copy link
Contributor

@wjwithagen could you please amend your commit message?

@wjwithagen
Copy link
Contributor Author

@tchaikov
Fixed typos. Or did you have something else that needed amending?

@@ -227,7 +227,7 @@ void MemDB::MDBTransactionImpl::rmkey(const string &prefix,
dtrace << __func__ << " " << prefix << " " << k << dendl;
ops.push_back(make_pair(DELETE,
std::make_pair(std::make_pair(prefix, k),
NULL)));
bufferlist() )));
Copy link
Contributor

Choose a reason for hiding this comment

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

could remove the space after bufferlist()

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@tchaikov
Did that on purpose, to separate the function end from the other )s.
But it does not conform the standard.

@tchaikov
Copy link
Contributor

tchaikov commented Jul 28, 2016

@wjwithagen i explained the reason why this piece of code failed to compile in the reply to your mail, and put a shorter version of it in the comment of #10455 (comment), i'd recommend you to correct the commit message. as your explanation is sort of misleading.

also the typo in your commit message was not fixed (s/strick/strict/).

@wjwithagen
Copy link
Contributor Author

retest this please

…tch the ops's value_type

 - NULL is a C-type usage, which is allow by GCC.
   But Clang is more strict in checking.

Error looks like:
src/kv/MemDB.cc:228: error: no matching member function
for call to 'push_back'
  ops.push_back(make_pair(DELETE,
  ~~~~^~~~~~~~~
/usr/include/c++/v1/vector:685:36: note: candidate function not viable:
no known conversion from 'pair<[...], pair<[...], nullptr_t>>' to
      'const pair<[...], pair<[...], ceph::buffer::list>>' for 1st argument
    _LIBCPP_INLINE_VISIBILITY void push_back(const_reference __x);
                                   ^

Signed-off-by: Willem Jan Withagen <wjw@digiware.nl>
@wjwithagen wjwithagen changed the title src/kv/MemDB.cc: Clang compains about using NULL as empty bufferlist. src/kv/MemDB.cc: the type of the parameter of push_back() does not match the ops's value_type Jul 28, 2016
@@ -227,7 +227,7 @@ void MemDB::MDBTransactionImpl::rmkey(const string &prefix,
dtrace << __func__ << " " << prefix << " " << k << dendl;
ops.push_back(make_pair(DELETE,
std::make_pair(std::make_pair(prefix, k),
NULL)));
bufferlist())));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Looks good to me.

@wjwithagen
Copy link
Contributor Author

@tchaikov
I think this one is also good to go?

@tchaikov tchaikov merged commit f4085aa into ceph:master Aug 10, 2016
@tchaikov tchaikov self-assigned this Aug 10, 2016
@wjwithagen wjwithagen deleted the wip-wjw-clang-NULL branch January 23, 2019 12:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
3 participants