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/rbd: silence warning from -Wunused-variable #16670

Merged
merged 1 commit into from Jul 31, 2017

Conversation

Projects
None yet
3 participants
@Yan-waller
Contributor

Yan-waller commented Jul 29, 2017

[ 31%] Building CXX object src/test/librados_test_stub/CMakeFiles/rados_test_stub.dir/TestMemIoCtxImpl.cc.o
/home/jenkins-build/build/workspace/ceph-pull-requests/src/cls/rbd/cls_rbd.cc: In function ‘int trash_list(cls_method_context_t, ceph::bufferlist*, ceph::bufferlist*)’:
/home/jenkins-build/build/workspace/ceph-pull-requests/src/cls/rbd/cls_rbd.cc:5049:7: warning: unused variable ‘max_read’ [-Wunused-variable]
   int max_read = RBD_MAX_KEYS_READ;

Signed-off-by: Yan Jun yan.jun8@zte.com.cn

@joscollin joscollin changed the title from rbd/cls: kill compile warning to cls/rbd: silence warning from -Wunused-variable Jul 29, 2017

int max_read = std::min<int32_t>(RBD_MAX_KEYS_READ,
max_return - data.size());
max_read = std::min<int32_t>(RBD_MAX_KEYS_READ,
max_return - data.size());
int r = cls_cxx_map_get_vals(hctx, last_read, trash::IMAGE_KEY_PREFIX,

This comment has been minimized.

@joscollin

joscollin Jul 29, 2017

Member

Now why assigning RBD_MAX_KEYS_READ to max_read at L5049 ?

This comment has been minimized.

@Yan-waller

Yan-waller Jul 29, 2017

Contributor

@joscollin , reference to PR #16667:5049 , since max_return could be specified from client, max_read will be reassigned at L5055. so... fix this by dropping L5049 seems to be better, what do you think?

This comment has been minimized.

@joscollin

joscollin Jul 29, 2017

Member

I think you can keep the declaration of max_read outside the while loop, without issues.

This comment has been minimized.

@Yan-waller

Yan-waller Jul 31, 2017

Contributor

@joscollin okay done, thank you

This comment has been minimized.

@joscollin

joscollin Jul 31, 2017

Member

I meant like this. There is no reason for the assignment, as it is done immediately after entering the while loop. The other branch not entering the while loop doesn't use max_read too.

int max_read;
bool more = true;

  CLS_LOG(20, "trash_get_images");
  while (data.size() < max_return) {
    map<string, bufferlist> raw_data;
    max_read = std::min<int32_t>(RBD_MAX_KEYS_READ,
                                 max_return - data.size());

This comment has been minimized.

@Yan-waller

Yan-waller Jul 31, 2017

Contributor

OK, I see.

@joscollin

Looks Good to me.

@joscollin joscollin added the needs-qa label Jul 31, 2017

@joscollin

This comment has been minimized.

Member

joscollin commented Jul 31, 2017

Jenkins retest this please

bool more = true;
CLS_LOG(20, "trash_get_images");
while (data.size() < max_return) {
map<string, bufferlist> raw_data;
int max_read = std::min<int32_t>(RBD_MAX_KEYS_READ,
max_return - data.size());
max_read = std::min<int32_t>(RBD_MAX_KEYS_READ,

This comment has been minimized.

@dillaman

dillaman Jul 31, 2017

Contributor

Keep the variable scoping as tight as possible -- keep the original line here and just drop line 5049 since it was being shadowed.

This comment has been minimized.

@Yan-waller

Yan-waller Jul 31, 2017

Contributor

@joscollin I think we can silence this warning by any of these two ways, but this comment from dillaman sounds reasonable, so I resubmited it, is that OK? anyways, thanks you two.

cls/rbd: silence warning from -Wunused-variable
Signed-off-by: Yan Jun <yan.jun8@zte.com.cn>

@dillaman dillaman merged commit 41c251c into ceph:master Jul 31, 2017

3 of 4 checks passed

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
make check make check succeeded
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment