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

core: make the conversion from wire error to host OS work #15780

Merged
merged 1 commit into from Jul 5, 2017

Conversation

Projects
None yet
3 participants
@wjwithagen
Contributor

wjwithagen commented Jun 20, 2017

  • The key change is the type of rval,
    that will call the conversion when en/decoded
  • Remainder is fixes for the type change

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

@wjwithagen wjwithagen requested a review from liewegas Jun 20, 2017

@@ -1154,7 +1154,7 @@ struct ObjectOperation {
for (uint32_t i = 0; i < sops.size(); i++) {
out_bl[i] = &sops[i].outdata;
out_handler[i] = NULL;
out_rval[i] = &sops[i].rval;
out_rval[i] = (int*)(&sops[i].rval);

This comment has been minimized.

@liewegas

liewegas Jun 20, 2017

Member

int32_t instead of int

This comment has been minimized.

@wjwithagen

wjwithagen Jun 20, 2017

Contributor

@liewegas
Right, that si more to the point.
The debugging code cruft is removed.

This comment has been minimized.

@wjwithagen

wjwithagen Jun 20, 2017

Contributor

@liewegas
Clang complains about conversion when there is no explicit cast to (int32*).
So either we add a a function that helps Clang, or we just cast?

@liewegas

This comment has been minimized.

Member

liewegas commented Jun 20, 2017

I don't think the objecter changes are needed (the debugging noise or the explicit conversion); the error codes should be fixed up when MOSDOpReply is encoded or decoded.

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jun 21, 2017

@liewegas
I think this PR is now as it should be like you asked. And the jenkins error seems to be unrelated

@liewegas liewegas added the needs-qa label Jun 21, 2017

@wjwithagen

This comment has been minimized.

Contributor

wjwithagen commented Jul 1, 2017

@liewegas
Hi Sage, anything holding back merging this?

@liewegas

This comment has been minimized.

Member

liewegas commented Jul 2, 2017

just needs to get tested, should be in teh next batch

int operator==(int i) { return code==i; }
int operator>(int i) { return code>i; }
int operator>=(int i) { return code>=i; }
int operator<(int i) { return code>i; }

This comment has been minimized.

This comment has been minimized.

@wjwithagen

wjwithagen Jul 5, 2017

Contributor

@tchaikov
Uploaded the fixes.

}
operator int() const { return code; }
int* operator&() { return &code; }
int operator==(int i) { return code==i; }

This comment has been minimized.

@tchaikov

tchaikov Jul 5, 2017

Contributor

please add spaces around the binary operator.

@tchaikov tchaikov added wip-kefu-testing and removed needs-qa labels Jul 5, 2017

@tchaikov

This comment has been minimized.

Contributor

tchaikov commented Jul 5, 2017

being tested with proposed change.

core: make the conversion from wire error to host OS work
 - The key change is the type of rval,
   that will call the conversion when en/decoded
 - Remainder is fixes for the type change and promotions

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

@tchaikov tchaikov added the needs-qa label Jul 5, 2017

@tchaikov tchaikov merged commit 6cc0122 into ceph:master Jul 5, 2017

2 of 4 checks passed

make check running make check
Details
make check (arm64) running make check
Details
Signed-off-by all commits in this PR are signed
Details
Unmodified Submodules submodules for project are unmodified
Details

@wjwithagen wjwithagen deleted the wjwithagen:wip-wjw-freebsd-use-ceph_to_hostos_errno branch Feb 11, 2018

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