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.

Sign up for GitHub

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

Jump to bottom

DAOS-6732: Remove line of code that is not reached #4797

Merged
merged 6 commits into from
Mar 10, 2021
Merged

DAOS-6732: Remove line of code that is not reached #4797

merged 6 commits into from
Mar 10, 2021

Conversation

marcelarosalesj
Copy link
Contributor

This bug was identified by Coverity

Signed-off-by: Marcela Rosales marcela.a.rosales.jimenez@intel.com

@marcelarosalesj
DAOS-6732: Remove line of code that is not reached
7b805ea 
This bug was identified by Coverity

Signed-off-by: Marcela Rosales <marcela.a.rosales.jimenez@intel.com>
daosbuild1
daosbuild1 reviewed Feb 26, 2021
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

jolivier23
jolivier23 requested changes Feb 26, 2021
View reviewed changes
Copy link
Contributor

@jolivier23 jolivier23 left a comment

Choose a reason for hiding this comment

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

What is the point of st at all here? I think we should probably take st as an argument to ts_cmd_run and pass it to the other routines. Either that or we should get rid of it altogether if it's not needed to pass it to subroutines for a given test.

It looks like there are two calls, one from cmocka and one not...we could pass NULL to the other one but just pass state to the cmocka version.

@daosbuild1
Copy link
Collaborator

Test stage Functional_Hardware_Large completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4797/1/execution/node/1529/log

@marcelarosalesj
Remove usage of st argument
3dc5ee2 
Signed-off-by: Marcela Rosales <marcela.a.rosales.jimenez@intel.com>
daosbuild1
daosbuild1 reviewed Mar 1, 2021
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@marcelarosalesj
Copy link
Contributor Author

What is the point of st at all here? I think we should probably take st as an argument to ts_cmd_run and pass it to the other routines. Either that or we should get rid of it altogether if it's not needed to pass it to subroutines for a given test.

It looks like there are two calls, one from cmocka and one not...we could pass NULL to the other one but just pass state to the cmocka version.

Thanks for your feedback @jolivier23. I just sent a patch removing the st fom the other functions because I don't think it is necessary to have it. I reviewed the cmocka documentation and void **state is used for CMUnitTest test cases, and after #2364, the ts_* functions are not that kind of test cases anymore.

@daosbuild1
Copy link
Collaborator

Test stage Functional_Hardware_Small completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4797/2/execution/node/1272/log

@daosbuild1
Copy link
Collaborator

Test stage Functional_Hardware_Medium completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4797/2/execution/node/1301/log

@daosbuild1
Copy link
Collaborator

Test stage Functional_Hardware_Large completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4797/2/execution/node/1393/log

@daosbuild1
Copy link
Collaborator

Test stage Functional on CentOS 7 completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4797/2/execution/node/1526/log

daosbuild1
daosbuild1 reviewed Mar 2, 2021
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

jolivier23
jolivier23 requested changes Mar 2, 2021
View reviewed changes
src/vos/tests/evt_ctl.c Outdated
@@ -121,7 +121,7 @@ static struct evt_desc_cbs ts_evt_desc_nofree_cbs = {
};

static void
ts_open_create(void **state)
ts_open_create()
Copy link
Contributor

Choose a reason for hiding this comment

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

should be ts_open_create(void)

Same comment for other functions definitions. This ensures a compiler error if user actually tries to pass a parameter.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

okay, I'm sending the patch

@marcelarosalesj
Merge branch 'master' into marosale/DAOS-6732
751a99e
@marcelarosalesj
Use void as argument
964ad04 
Signed-off-by: Marcela Rosales <marcela.a.rosales.jimenez@intel.com>
daosbuild1
daosbuild1 reviewed Mar 3, 2021
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@daosbuild1
Copy link
Collaborator

Test stage Scan CentOS 7 RPMs completed with status FAILURE. https://build.hpdd.intel.com//job/daos-stack/job/daos/view/change-requests/job/PR-4797/4/execution/node/1181/log

daosbuild1
daosbuild1 reviewed Mar 8, 2021
View reviewed changes
Copy link
Collaborator

@daosbuild1 daosbuild1 left a comment

Choose a reason for hiding this comment

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

LGTM. No errors found by checkpatch.

@marcelarosalesj marcelarosalesj requested a review from a team March 9, 2021 16:30
@jolivier23 jolivier23 merged commit c569c2a into master Mar 10, 2021
@jolivier23 jolivier23 deleted the marosale/DAOS-6732 branch March 10, 2021 00:56
marcelarosalesj added a commit that referenced this pull request Mar 10, 2021
@marcelarosalesj
DAOS-6732: Remove line of code that is not reached (#4797)
b355fd6 
This bug was identified by Coverity

Signed-off-by: Marcela Rosales <marcela.a.rosales.jimenez@intel.com>
jolivier23 pushed a commit that referenced this pull request Mar 14, 2021
@marcelarosalesj
DAOS-6732 coverity: Remove line of code that is not reached (#4797) (#…
a87360a 
…4942)

This bug was identified by Coverity

Signed-off-by: Marcela Rosales <marcela.a.rosales.jimenez@intel.com>
@ashleypittman ashleypittman mentioned this pull request Apr 28, 2021
Closed
@ashleypittman ashleypittman mentioned this pull request May 20, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@daosbuild1 daosbuild1 daosbuild1 left review comments

@NiuYawei NiuYawei NiuYawei approved these changes

@jolivier23 jolivier23 jolivier23 approved these changes

@ryon-jensen ryon-jensen ryon-jensen approved these changes

@gnailzenh gnailzenh Awaiting requested review from gnailzenh

@rpadma2 rpadma2 Awaiting requested review from rpadma2

@ashleypittman ashleypittman Awaiting requested review from ashleypittman

@Nasf-Fan Nasf-Fan Awaiting requested review from Nasf-Fan

Assignees
No one assigned
Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@marcelarosalesj @daosbuild1 @NiuYawei @jolivier23 @ryon-jensen