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

test/librbd/fsx: Add break in case OP_WRITESAME and OP_COMPARE_AND_WRITE #16742

Merged
merged 1 commit into from Aug 4, 2017

Conversation

Projects
None yet
4 participants
@scienceluo
Copy link
Contributor

scienceluo commented Aug 2, 2017

Add break in case OP_WRITESAME and OP_COMPARE_AND_WRITE

Signed-off-by: Luo Kexue luo.kexue@zte.com.cn

@joscollin
Copy link
Member

joscollin left a comment

LGTM

goto out;
}
break;
case OP_COMPARE_AND_WRITE:

This comment has been minimized.

Copy link
@tchaikov

tchaikov Aug 2, 2017

Contributor

please avoid formatting only change unless you are touching this piece of code. this blurs the git history.

This comment has been minimized.

Copy link
@scienceluo

scienceluo Aug 2, 2017

Author Contributor

Sorry about this.
Don't know why it was mess after I push them up.
I will fix this later. Thanks @tchaikov

This comment has been minimized.

Copy link
@joscollin

joscollin Aug 2, 2017

Member

@scienceluo Yeah, I was wondering why the whole block shows up in green. But the resulting code was correct.

This comment has been minimized.

Copy link
@scienceluo

scienceluo Aug 2, 2017

Author Contributor

Ok, done. @tchaikov @joscollin

This comment has been minimized.

Copy link
@scienceluo

scienceluo Aug 2, 2017

Author Contributor

@tchaikov Ok with this?

This comment has been minimized.

Copy link
@joscollin

joscollin Aug 3, 2017

Member

@scienceluo Looks good now.

This comment has been minimized.

Copy link
@scienceluo

scienceluo Aug 3, 2017

Author Contributor

Cool. Thanks man. @joscollin

@scienceluo scienceluo force-pushed the scienceluo:wip-luo-coverity-fix-branch branch from 30412cf to 6116940 Aug 2, 2017

@tchaikov tchaikov added the tests label Aug 2, 2017

case OP_COMPARE_AND_WRITE:
/* compare_and_write not implemented */
if (!ops->compare_and_write) {
log4(OP_SKIPPED, OP_COMPARE_AND_WRITE, offset, size);
goto out;
}
break;
default:

This comment has been minimized.

Copy link
@dillaman

dillaman Aug 3, 2017

Contributor

Nit: I would leave off the default case so that the compilation will fail if a new enum is added and the switch statement isn't updated.

This comment has been minimized.

Copy link
@scienceluo

scienceluo Aug 4, 2017

Author Contributor

@dillaman Ok, I see.

So, like you said, shall we remove the default case in L2457 too?

This comment has been minimized.

Copy link
@joscollin

joscollin Aug 4, 2017

Member

@dillaman
Yeah! -Wswitch

This comment has been minimized.

Copy link
@joscollin

joscollin Aug 4, 2017

Member

@scienceluo
But in L2457, as per the current implementation, the default case handles the newly added enum as an unknown operation.

This comment has been minimized.

Copy link
@scienceluo

scienceluo Aug 4, 2017

Author Contributor

@dillaman @joscollin Ok, done. But I'm not sure if @tchaikov ok whis this.

This comment has been minimized.

Copy link
@joscollin

joscollin Aug 4, 2017

Member

@dillaman Do we need to leave a comment about why there is no default at L2404, so that some others won't introduce a default:, without knowing, in the future ?

This comment has been minimized.

Copy link
@scienceluo

scienceluo Aug 4, 2017

Author Contributor

@scienceluo scienceluo force-pushed the scienceluo:wip-luo-coverity-fix-branch branch 2 times, most recently from 80c84ac to e33ec7c Aug 4, 2017

test/librbd/fsx: Add break in case OP_WRITESAME and OP_COMPARE_AND_WRITE
Signed-off-by: Luo Kexue <luo.kexue@zte.com.cn>
@dillaman
Copy link
Contributor

dillaman left a comment

lgtm

@dillaman dillaman merged commit cb9af1d into ceph:master Aug 4, 2017

4 checks passed

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

@scienceluo scienceluo deleted the scienceluo:wip-luo-coverity-fix-branch branch Aug 7, 2017

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.