Skip to content

increase warning level on C/C++ compiler#1689

Merged
BareosBot merged 7 commits intobareos:masterfrom
arogge:dev/arogge/master/spring-cleanup
Mar 12, 2024
Merged

increase warning level on C/C++ compiler#1689
BareosBot merged 7 commits intobareos:masterfrom
arogge:dev/arogge/master/spring-cleanup

Conversation

@arogge
Copy link
Member

@arogge arogge commented Jan 29, 2024

This PR adds new warnings and refactors code where needed

  • enable -Wshadow and refactor problematic code
  • enable -Wnon-virtual-dtor

Please check

  • Short description and the purpose of this PR is present above this paragraph
  • Your name is present in the AUTHORS file (optional)

If you have any questions or problems, please give a comment in the PR.

Helpful documentation and best practices

Checklist for the reviewer of the PR (will be processed by the Bareos team)

Make sure you check/merge the PR using devtools/pr-tool to have some simple automated checks run and a proper changelog record added.

General
  • Is the PR title usable as CHANGELOG entry?
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
    Check backport line
    Required backport PRs have been created
Source code quality
  • Source code changes are understandable
  • Variable and function names are meaningful
  • Code comments are correct (logically and spelling)
  • Required documentation changes are present and part of the PR

@arogge arogge force-pushed the dev/arogge/master/spring-cleanup branch 3 times, most recently from 7767d56 to d3f4e9c Compare January 31, 2024 11:32
@arogge arogge force-pushed the dev/arogge/master/spring-cleanup branch from 481c0ba to 78bf5df Compare February 6, 2024 12:51
@arogge arogge force-pushed the dev/arogge/master/spring-cleanup branch 2 times, most recently from 5def8a2 to b2e994b Compare February 15, 2024 16:06
@arogge arogge changed the title Spring Cleanup 2024 increase warning level on C/C++ compiler Feb 15, 2024
@arogge arogge marked this pull request as ready for review February 15, 2024 16:07
@sebsura sebsura self-requested a review February 16, 2024 08:18
Copy link
Contributor

@sebsura sebsura left a comment

Choose a reason for hiding this comment

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

I like the new warnings, but in general i feel like we should not rename local variables of functions but instead make it clearer that a global variable is global by renaming the global variable.

@arogge
Copy link
Member Author

arogge commented Feb 29, 2024

Please take another look. I have addressed most of your requested changes and left comments on the others.

Copy link
Contributor

@sebsura sebsura left a comment

Choose a reason for hiding this comment

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

Thanks for the changes. I added some comments as well as adding some comments to the previous (open) conversations as well.

Copy link
Contributor

@sebsura sebsura left a comment

Choose a reason for hiding this comment

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

Thanks for the work! Looks good to me!

@arogge arogge force-pushed the dev/arogge/master/spring-cleanup branch from 248003e to 290eca5 Compare March 8, 2024 08:09
@sebsura sebsura force-pushed the dev/arogge/master/spring-cleanup branch from 290eca5 to a5b9c64 Compare March 8, 2024 11:09
arogge and others added 7 commits March 12, 2024 15:41
and refactor all code that triggers that warning.
As there is no way to set g_verbose to anything but 0 or 1 in dbcheck,
this code is unrechable and can be removed safely.
this function can simplify the current use of FindArgWithValue(). It
does the same thing, but instead of returning the index, it returns a
pointer to the value, or nullptr if the argument was not found or did
not have a value.
@BareosBot BareosBot force-pushed the dev/arogge/master/spring-cleanup branch from 5579484 to ab10dbd Compare March 12, 2024 15:41
@BareosBot BareosBot merged commit 21a1a23 into bareos:master Mar 12, 2024
pstorz pushed a commit to pstorz/bareos that referenced this pull request Dec 21, 2024
Commit PR bareos#1689 (812f9e4) contains a change in core/src/lib/crypto.h, which has only been partially synced with core/src/lib/crypto_openssl.cc. The latter file contains two definitions of EvpDigest(), depending on OpenSSL major version. Only one was fixed.

When compiling e.g. on CentOS 7 one gets an error about shadowing.

(cherry picked from commit a85a409)
pstorz pushed a commit to pstorz/bareos that referenced this pull request Dec 21, 2024
Commit PR bareos#1689 (812f9e4) contains a
change in core/src/lib/crypto.h, which has only been partially synced
with core/src/lib/crypto_openssl.cc. The latter file contains two
definitions of EvpDigest(), depending on OpenSSL major version. Only one
was fixed.

When compiling e.g. on CentOS 7 one gets an error about shadowing.

(cherry picked from commit a85a409)
florian-at-bareos pushed a commit to florian-at-bareos/bareos that referenced this pull request Jan 27, 2025
Commit PR bareos#1689 (812f9e4) contains a
change in core/src/lib/crypto.h, which has only been partially synced
with core/src/lib/crypto_openssl.cc. The latter file contains two
definitions of EvpDigest(), depending on OpenSSL major version. Only one
was fixed.

When compiling e.g. on CentOS 7 one gets an error about shadowing.
florian-at-bareos pushed a commit to florian-at-bareos/bareos that referenced this pull request Jan 27, 2025
Commit PR bareos#1689 (812f9e4) contains a
change in core/src/lib/crypto.h, which has only been partially synced
with core/src/lib/crypto_openssl.cc. The latter file contains two
definitions of EvpDigest(), depending on OpenSSL major version. Only one
was fixed.

When compiling e.g. on CentOS 7 one gets an error about shadowing.
pstorz pushed a commit to pstorz/bareos that referenced this pull request Jan 30, 2025
Commit PR bareos#1689 (812f9e4) contains a
change in core/src/lib/crypto.h, which has only been partially synced
with core/src/lib/crypto_openssl.cc. The latter file contains two
definitions of EvpDigest(), depending on OpenSSL major version. Only one
was fixed.

When compiling e.g. on CentOS 7 one gets an error about shadowing.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants