Skip to content

Fix gcc warnings in ndmjob program#1343

Merged
arogge merged 2 commits intobareos:masterfrom
tuxmaster5000:GCC_fix_Fedora
Jan 19, 2023
Merged

Fix gcc warnings in ndmjob program#1343
arogge merged 2 commits intobareos:masterfrom
tuxmaster5000:GCC_fix_Fedora

Conversation

@tuxmaster5000
Copy link
Contributor

@tuxmaster5000 tuxmaster5000 commented Dec 30, 2022 •
edited by arogge
Loading

On Fedora 37 the gcc will report many warnings as errors, so the warnings need an fix.

cd /builddir/build/BUILD/bareos-Release-22.0.0/redhat-linux-build/core/src/ndmp && ccache /usr/lib64/ccache/gcc -D_FILE_OFFSET_BITS=64 -I/usr/include/tirpc -I/builddir/build/BUILD/bareos-Release-22.0.0/core/src -I/builddir/build/BUILD/bareos-Release-22.0.0/redhat-linux-build/core/src/ndmp -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong -specs=/usr/lib/rpm/redhat/redhat-annobin-cc1  -m64  -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -Wno-error=maybe-uninitialized -Wno-error=format-truncation -fdebug-prefix-map=/builddir/build/BUILD/bareos-Release-22.0.0/core=. -fmacro-prefix-map=/builddir/build/BUILD/bareos-Release-22.0.0/core=. -Werror -Wall -Wextra -DNDMOS_OPTION_NO_TEST_AGENTS -DNDMOS_CONST_VENDOR_NAME=\"Bareos\ GmbH\ \&\ Co.KG\" -DNDMOS_CONST_PRODUCT_NAME=\"Bareos\" -DHAVE_LINUX_OS -MD -MT core/src/ndmp/CMakeFiles/ndmjob.dir/ndmjr_none.c.o -MF CMakeFiles/ndmjob.dir/ndmjr_none.c.o.d -o CMakeFiles/ndmjob.dir/ndmjr_none.c.o -c /builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjr_none.c
ccache /usr/bin/ar qc libstored_objects.a CMakeFiles/stored_objects.dir/append.cc.o CMakeFiles/stored_objects.dir/askdir.cc.o CMakeFiles/stored_objects.dir/authenticate.cc.o CMakeFiles/stored_objects.dir/dir_cmd.cc.o CMakeFiles/stored_objects.dir/fd_cmds.cc.o CMakeFiles/stored_objects.dir/job.cc.o CMakeFiles/stored_objects.dir/mac.cc.o CMakeFiles/stored_objects.dir/ndmp_tape.cc.o CMakeFiles/stored_objects.dir/read.cc.o CMakeFiles/stored_objects.dir/sd_cmds.cc.o CMakeFiles/stored_objects.dir/sd_stats.cc.o CMakeFiles/stored_objects.dir/socket_server.cc.o CMakeFiles/stored_objects.dir/status.cc.o
ccache /usr/bin/ranlib libstored_objects.a
gmake[2]: Leaving directory '/builddir/build/BUILD/bareos-Release-22.0.0/redhat-linux-build'
[ 55%] Built target stored_objects
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjr_none.c: In function 'ndmjr_none_apply':
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjr_none.c:46:44: error: unused parameter 'job' [-Werror=unused-parameter]
   46 | int ndmjr_none_apply(struct ndm_job_param* job, char* reason) { return 0; }
      |                      ~~~~~~~~~~~~~~~~~~~~~~^~~
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjr_none.c:46:55: error: unused parameter 'reason' [-Werror=unused-parameter]
   46 | int ndmjr_none_apply(struct ndm_job_param* job, char* reason) { return 0; }
      |                                                 ~~~~~~^~~~~~
cc1: all warnings being treated as errors
gmake[2]: *** [core/src/ndmp/CMakeFiles/ndmjob.dir/build.make:177: core/src/ndmp/CMakeFiles/ndmjob.dir/ndmjr_none.c.o] Error 1
gmake[2]: *** Waiting for unfinished jobs....
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_main_util.c: In function 'ndmjob_log_deliver':
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_main_util.c:97:40: error: unused parameter 'log' [-Werror=unused-parameter]
   97 | void ndmjob_log_deliver(struct ndmlog* log, char* tag, int lev, char* msg)
      |                         ~~~~~~~~~~~~~~~^~~
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_main_util.c: In function 'ndmjob_ixlog_deliver':
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_main_util.c:126:42: error: unused parameter 'log' [-Werror=unused-parameter]
  126 | void ndmjob_ixlog_deliver(struct ndmlog* log, char* tag, int lev, char* msg)
      |                           ~~~~~~~~~~~~~~~^~~
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_main_util.c:126:62: error: unused parameter 'lev' [-Werror=unused-parameter]
  126 | void ndmjob_ixlog_deliver(struct ndmlog* log, char* tag, int lev, char* msg)
      |                                                          ~~~~^~~
cc1: all warnings being treated as errors
gmake[2]: *** [core/src/ndmp/CMakeFiles/ndmjob.dir/build.make:135: core/src/ndmp/CMakeFiles/ndmjob.dir/ndmjob_main_util.c.o] Error 1
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_args.c: In function 'copy_args_expanding_macros':
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_args.c:826:72: error: unused parameter 'max_ac' [-Werror=unused-parameter]
  826 | int copy_args_expanding_macros(int argc, char* argv[], char* av[], int max_ac)
      |                                                                    ~~~~^~~~~~
cc1: all warnings being treated as errors
gmake[2]: *** [core/src/ndmp/CMakeFiles/ndmjob.dir/build.make:79: core/src/ndmp/CMakeFiles/ndmjob.dir/ndmjob_args.c.o] Error 1
gmake[2]: Leaving directory '/builddir/build/BUILD/bareos-Release-22.0.0/redhat-linux-build'
[ 55%] Built target bareossd-droplet
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_simulator.c: In function 'robot_state_load':
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_simulator.c:960:40: error: comparison of integer expressions of different signedness: 'ssize_t' {aka 'long int'} and 'long unsigned int' [-Werror=sign-compare]
  960 |   if (read(fd, (void*)rs, sizeof(*rs)) < sizeof(*rs)) {
      |                                        ^
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_simulator.c: In function 'robot_state_save':
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_simulator.c:980:41: error: comparison of integer expressions of different signedness: 'ssize_t' {aka 'long int'} and 'long unsigned int' [-Werror=sign-compare]
  980 |   if (write(fd, (void*)rs, sizeof(*rs)) < sizeof(*rs)) {
      |                                         ^
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_simulator.c: In function 'ndmjob_scsi_close':
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_simulator.c:1465:58: error: unused parameter 'sess' [-Werror=unused-parameter]
 1465 | static ndmp9_error ndmjob_scsi_close(struct ndm_session* sess)
      |                                      ~~~~~~~~~~~~~~~~~~~~^~~~
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_simulator.c: In function 'ndmjob_scsi_reset':
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_simulator.c:1470:58: error: unused parameter 'sess' [-Werror=unused-parameter]
 1470 | static ndmp9_error ndmjob_scsi_reset(struct ndm_session* sess)
      |                                      ~~~~~~~~~~~~~~~~~~~~^~~~
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_simulator.c: In function 'ndmjob_validate_password':
/builddir/build/BUILD/bareos-Release-22.0.0/core/src/ndmp/ndmjob_simulator.c:1492:57: error: unused parameter 'sess' [-Werror=unused-parameter]
 1492 | static int ndmjob_validate_password(struct ndm_session* sess,
      |                                     ~~~~~~~~~~~~~~~~~~~~^~~~
cc1: all warnings being treated as errors
gmake[2]: *** [core/src/ndmp/CMakeFiles/ndmjob.dir/build.make:163: core/src/ndmp/CMakeFiles/ndmjob.dir/ndmjob_simulator.c.o] Error 1
gmake[2]: Leaving directory '/builddir/build/BUILD/bareos-Release-22.0.0/redhat-linux-build'
gmake[1]: *** [CMakeFiles/Makefile2:3331: core/src/ndmp/CMakeFiles/ndmjob.dir/all] Error 2
gmake[1]: *** Waiting for unfinished jobs....

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)

General
  • PR name is meaningful
  • Purpose of the PR is understood
  • Commit descriptions are understandable and well formatted
  • Check backport line
  • Is the PR title usable as CHANGELOG entry?
  • Separate commit for CHANGELOG.md ("update CHANGELOG.md"). The PR number is correct.
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
  • bareos-check-sources --since-merge does not report any problems

@tuxmaster5000 tuxmaster5000 changed the title Fix gcc erros on Fedora-37 Fix gcc errors on Fedora-37 Dec 30, 2022
arogge
arogge requested changes Jan 12, 2023
View reviewed changes
Copy link
Member

@arogge arogge left a comment

Choose a reason for hiding this comment

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

Hi!
thank you very much for taking the time to fix this. The ndmjob program is largely unmaintained, so it doesn't get much attention.
The [[maybe_unused]] attributes are pretty obvious. Your (ssize_t) cast looks wrong: you need to cast the return value check instead of the parameter passed to write().

You can also remove your changelog record and rename the PR accordingly, the changelog will be now be updated a merge-time using the PR title so you don't have to take care of that anymore.

@arogge
Copy link
Member

arogge commented Jan 12, 2023

I just tried to build it and realized NDMP is built with a plain C compiler, so [[maybe_unused]] will not work.
As you cannot mark unused parameters as such in C, you'll need to do something like this:

int some_func(int x) {
  (void)x; // unused
}

At least that seems to build fine for me.

Also, with our default warning levels I didn't get a warning/problem when comparing the ssize_t return code of read() or write() with a size_t.

@tuxmaster5000
Copy link
Contributor Author

tuxmaster5000 commented Jan 13, 2023

I have modify it, please take an look again for it.

@arogge
Copy link
Member

arogge commented Jan 13, 2023

Still doesn't build on my machine:

FAILED: core/src/ndmp/CMakeFiles/ndmjob.dir/ndmjob_main_util.c.o
ccache /usr/bin/cc -D_FILE_OFFSET_BITS=64 -I/usr/include/tirpc -Icore/src -Icore/src/ndmp -O2 -g -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -fexceptions -fstack-protector-strong -grecord-gcc-switches -m64 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -fPIC -fsanitize=undefined -fsanitize=address -fno-sanitize-recover -fdebug-prefix-map=/home/arogge/workspace/bareos-b/core=. -fmacro-prefix-map=/home/arogge/workspace/bareos-b/core=. -Werror -Wall -Wextra -DNDMOS_OPTION_NO_TEST_AGENTS -DNDMOS_CONST_VENDOR_NAME=\"Bareos\ GmbH\ \&\ Co.KG\" -DNDMOS_CONST_PRODUCT_NAME=\"Bareos\" -DHAVE_LINUX_OS -MD -MT core/src/ndmp/CMakeFiles/ndmjob.dir/ndmjob_main_util.c.o -MF core/src/ndmp/CMakeFiles/ndmjob.dir/ndmjob_main_util.c.o.d -o core/src/ndmp/CMakeFiles/ndmjob.dir/ndmjob_main_util.c.o -c /home/arogge/workspace/bareos-b/core/src/ndmp/ndmjob_main_util.c
core/src/ndmp/ndmjob_main_util.c: In function 'ndmjob_log_deliver':
core/src/ndmp/ndmjob_main_util.c:99:8: error: expected identifier or '(' before ')' token
   void() log; //unused
        ^
core/src/ndmp/ndmjob_main_util.c:97:40: error: unused parameter 'log' [-Werror=unused-parameter]
 void ndmjob_log_deliver(struct ndmlog* log, char* tag, int lev, char* msg)
                         ~~~~~~~~~~~~~~~^~~
core/src/ndmp/ndmjob_main_util.c: In function 'ndmjob_ixlog_deliver':
core/src/ndmp/ndmjob_main_util.c:129:8: error: expected identifier or '(' before ')' token
   void() log; //unused
        ^
core/src/ndmp/ndmjob_main_util.c:130:8: error: expected identifier or '(' before ')' token
   void() lev; //unused
        ^
core/src/ndmp/ndmjob_main_util.c:127:42: error: unused parameter 'log' [-Werror=unused-parameter]
 void ndmjob_ixlog_deliver(struct ndmlog* log, char* tag, int lev, char* msg)
                           ~~~~~~~~~~~~~~~^~~
core/src/ndmp/ndmjob_main_util.c:127:62: error: unused parameter 'lev' [-Werror=unused-parameter]
 void ndmjob_ixlog_deliver(struct ndmlog* log, char* tag, int lev, char* msg)
                                                          ~~~~^~~
cc1: all warnings being treated as errors
ninja: build stopped: subcommand failed.

@arogge arogge changed the title Fix gcc errors on Fedora-37 Fix gcc warnings on Fedora-37 Jan 16, 2023
@arogge arogge changed the title Fix gcc warnings on Fedora-37 Fix gcc warnings in ndmjob program Jan 16, 2023
@arogge
Copy link
Member

arogge commented Jan 16, 2023

Do you want to clean up the commits or should I do it?

@tuxmaster5000
Copy link
Contributor Author

tuxmaster5000 commented Jan 17, 2023

What do you mean with clean up?

@arogge
Copy link
Member

arogge commented Jan 17, 2023

In this case probably merging all the commits into one.

We usually try to have a "clean" git history (i.e. what it would have looked like if you got it 100% correct on the first try), because that is easier to understand for people who work through the git log in the future.

@tuxmaster5000
Copy link
Contributor Author

tuxmaster5000 commented Jan 17, 2023

Ah, I understood. But how can I do this here?

@arogge
Copy link
Member

arogge commented Jan 17, 2023

I don't think that's possible in Github. I always use git rebase and then force-push the results.

@arogge
Copy link
Member

arogge commented Jan 17, 2023

But as I wrote - I can also do that, if you want.

@tuxmaster5000
Copy link
Contributor Author

tuxmaster5000 commented Jan 19, 2023

Ok, then please do it.

@arogge arogge merged commit 8a2cbc3 into bareos:master Jan 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@arogge arogge arogge approved these changes

Assignees

@arogge arogge

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tuxmaster5000 @arogge