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

Jump to bottom

Correct cppcheck errors #6653

Merged
merged 1 commit into from Sep 19, 2017
Merged

Correct cppcheck errors #6653

merged 1 commit into from Sep 19, 2017

Conversation

dinatale2
Copy link
Contributor

Description

ZFS buildbot STYLE builder was moved to Ubuntu 17.04
which has a newer version of cppcheck. Handle the
new cppcheck errors.

Motivation and Context

Fix STYLE builder.

How Has This Been Tested?

Builds locally on a VM.

Types of changes

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Performance enhancement (non-breaking change which improves efficiency)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to man pages or other documentation)

Checklist:

  • My code follows the ZFS on Linux code style requirements.
  • I have updated the documentation accordingly.
  • I have read the CONTRIBUTING document.
  • I have added tests to cover my changes.
  • All new and existing tests passed.
  • All commit messages are properly formatted and contain Signed-off-by.
  • Change has been approved by a ZFS on Linux member.

@dinatale2 dinatale2 added the Status: Work in Progress Not yet ready for general review label Sep 18, 2017
@dinatale2 dinatale2 force-pushed the cppcheck-fixes branch 3 times, most recently from d2dac89 to e7241d0 Compare September 18, 2017 19:47
behlendorf
behlendorf reviewed Sep 18, 2017
View reviewed changes
module/zfs/vdev_raidz_math_avx512f.c Outdated
@@ -241,6 +241,7 @@ typedef struct v {
}

#define MUL_x2_DEFINE(x) \
/* cppcheck-suppress syntaxError */ \
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this may need to be,

// cppcheck-suppress syntaxError

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You're right. I was trying to figure out a way to get the supression comment to be included in the macro so cppcheck would actually suppress it and not break builds.

@behlendorf and I discussed offline and I'll have another attempted fix shortly.

@dinatale2 dinatale2 force-pushed the cppcheck-fixes branch 18 times, most recently from 4544d45 to 3ab5596 Compare September 19, 2017 00:25
@dinatale2
Copy link
Contributor Author

dinatale2 commented Sep 19, 2017 •
edited

For some reason, having preprocessorErrorDirective:filename:lineno won't work... but if I simply ignore all preprocessorErrorDirective in suppressions.txt, cppcheck functions as expected.

@behlendorf
Copy link
Contributor

That's unfortunate, then how about we exclude these two configs -UHAVE_SSE2 -UHAVE_AVX512F instead.

@codecov
Copy link

codecov bot commented Sep 19, 2017 •
edited

Codecov Report

❗ No coverage uploaded for pull request base (master@8e2ddda). Click here to learn what that means.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff            @@
##             master    #6653   +/-   ##
=========================================
  Coverage          ?   66.42%           
=========================================
  Files             ?      215           
  Lines             ?    71600           
  Branches          ?    14199           
=========================================
  Hits              ?    47560           
  Misses            ?    18391           
  Partials          ?     5649
Impacted Files Coverage Δ
module/zfs/zfs_vnops.c 49.75% <ø> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 8e2ddda...ed775d5. Read the comment docs.

@dinatale2 dinatale2 force-pushed the cppcheck-fixes branch 2 times, most recently from e051dc0 to abad7a6 Compare September 19, 2017 15:45
@dinatale2
Copy link
Contributor Author

dinatale2 commented Sep 19, 2017 •
edited

@behlendorf I went ahead and pinged them on their IRC channel and am waiting for a response. I submitted an updated commit to use -UHAVE_SSE2 -UHAVE_AVX512F in the meantime. If I get a response on IRC, I'll update it again.

I also left the two suppressions for the preprocessorErrorDirective errors in suppressions.txt as documentation since it appears not to harm anything to keep them in there.

behlendorf
behlendorf approved these changes Sep 19, 2017
View reviewed changes
Copy link
Contributor

@behlendorf behlendorf left a comment

Choose a reason for hiding this comment

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

It's a shame the suppression file didn't work, but practically speaking we can live with this. At least for now. I can extend the comment slightly in the merge to explain that both of these uu_* functions were unused and deadcode and therefore retired.

@dinatale2
Correct cppcheck errors
ed775d5 
ZFS buildbot STYLE builder was moved to Ubuntu 17.04
which has a newer version of cppcheck. Handle the
new cppcheck errors.

uu_* functions removed in this commit were unused
and effectively dead code. They are now retired.

Signed-off-by: Giuseppe Di Natale <dinatale2@llnl.gov>
@dinatale2 dinatale2 removed the Status: Work in Progress Not yet ready for general review label Sep 19, 2017
@behlendorf behlendorf merged commit 34d00e7 into openzfs:master Sep 19, 2017
@dinatale2 dinatale2 deleted the cppcheck-fixes branch September 19, 2017 22:01
dinatale2 added a commit to dinatale2/zfs that referenced this pull request Sep 20, 2017
@dinatale2
Correct cppcheck errors
549954e 
ZFS buildbot STYLE builder was moved to Ubuntu 17.04
which has a newer version of cppcheck. Handle the
new cppcheck errors.

uu_* functions removed in this commit were unused
and effectively dead code. They are now retired.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Closes openzfs#6653
tonyhutter pushed a commit that referenced this pull request Sep 20, 2017
@dinatale2 @tonyhutter
Correct cppcheck errors (#6662)
bef6a8b 
ZFS buildbot STYLE builder was moved to Ubuntu 17.04
which has a newer version of cppcheck. Handle the
new cppcheck errors.

uu_* functions removed in this commit were unused
and effectively dead code. They are now retired.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Closes #6653
Fabian-Gruenbichler pushed a commit to Fabian-Gruenbichler/zfs that referenced this pull request Sep 29, 2017
@dinatale2 @Fabian-Gruenbichler
Correct cppcheck errors (openzfs#6662)
95bdf87 
ZFS buildbot STYLE builder was moved to Ubuntu 17.04
which has a newer version of cppcheck. Handle the
new cppcheck errors.

uu_* functions removed in this commit were unused
and effectively dead code. They are now retired.

Reviewed-by: George Melikov <mail@gmelikov.ru>
Reviewed-by: Brian Behlendorf <behlendorf1@llnl.gov>
Signed-off-by: Giuseppe Di Natale <dinatale2@llnl.gov>
Closes openzfs#6653
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@behlendorf behlendorf behlendorf approved these changes

@gmelikov gmelikov gmelikov approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@dinatale2 @behlendorf @gmelikov