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

gcc-15 fixes - use void for sig_handler returns #3686

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

heitbaum
Copy link

@heitbaum heitbaum commented Dec 9, 2024

  • The Jira issue number for this PR is: MDEV-35607

Description

fixes compile issue with gcc-15

../mysys/stacktrace.c: In function 'default_handle_fatal_signal':
../mysys/stacktrace.c:53:3: error: 'return' with no value, in function 
returning non-void [-Wreturn-mismatch]
    53 |   return;
       |   ^~~~~~
...

Release Notes

Fixes build with gcc-15 when combined with fixes from:

How can this PR be tested?

Testing can be performed using gcc-15 as a compiler

Basing the PR against the correct MariaDB version

  • This is a new feature or a refactoring, and the PR is based against the main branch.
  • This is a bug fix, and the PR is based against the earliest maintained branch in which the bug can be reproduced.

PR quality check

  • I checked the CODING_STANDARDS.md file and my PR conforms to this where appropriate.
  • For any trivial modifications to the PR, I am ok with the reviewer making the changes themselves.

@CLAassistant
Copy link

CLAassistant commented Dec 9, 2024

CLA assistant check
All committers have signed the CLA.

@heitbaum heitbaum marked this pull request as draft December 9, 2024 07:07
@heitbaum heitbaum changed the title mysys: incorrect use of return mysys: use void for signal returns Dec 9, 2024
@grooverdan
Copy link
Member

Interesting - note it looks like it was intended to autodetect this -

CHECK_C_SOURCE_COMPILES("
- I haven't looked into the history as to why.

@heitbaum
Copy link
Author

heitbaum commented Dec 9, 2024

Interesting - note it looks like it was intended to autodetect this -

CHECK_C_SOURCE_COMPILES("

  • I haven't looked into the history as to why.

Yes. I'll have a look why once I get all the parts cleanly compiling. I thought it was going to be a quick one liner... :-(

@heitbaum heitbaum marked this pull request as ready for review December 9, 2024 08:40
@dr-m
Copy link
Contributor

dr-m commented Dec 9, 2024

Interesting - note it looks like it was intended to autodetect this -

CHECK_C_SOURCE_COMPILES("

  • I haven't looked into the history as to why.

If that auto-detection is obsolete by now, maybe we could remove it and thereby reduce the CMake execution time. man 2 signal on my system mentions C11 (ISO/IEC 9899:2011). In https://en.cppreference.com/w/c/program/signal there is no mention of signal() returning void; it always was supposed to return a function pointer. I would expect that any currently supported compiler would support at least C11.

fixes:
    ../mysys/stacktrace.c: In function 'default_handle_fatal_signal':
    ../mysys/stacktrace.c:53:3: error: 'return' with no value, in function
    returning non-void [-Wreturn-mismatch]
       53 |   return;
          |   ^~~~~~

Signed-off-by: Rudi Heitbaum <[email protected]>
@heitbaum heitbaum changed the title mysys: use void for signal returns mysys: use void for sig_handler returns Dec 9, 2024
@heitbaum
Copy link
Author

heitbaum commented Dec 9, 2024

Interesting - note it looks like it was intended to autodetect this -

CHECK_C_SOURCE_COMPILES("

  • I haven't looked into the history as to why.

If that auto-detection is obsolete by now, maybe we could remove it and thereby reduce the CMake execution time. man 2 signal on my system mentions C11 (ISO/IEC 9899:2011). In https://en.cppreference.com/w/c/program/signal there is no mention of signal() returning void; it always was supposed to return a function pointer. I would expect that any currently supported compiler would support at least C11.

That article concurs with the changes I have made:

void (*signal( int sig, void (*handler) (int))) (int);
                        ^^^^

In that the handler is suppose to be void not sig_handler which happens to be int in this instance.

I have updated the commit messages to reference sig_handler so as to be more correct.

@heitbaum heitbaum changed the title mysys: use void for sig_handler returns gcc-15 fixes - use void for sig_handler returns Dec 9, 2024
@cvicentiu cvicentiu added the External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements. label Dec 9, 2024
@cvicentiu cvicentiu self-assigned this Dec 9, 2024
@vuvova vuvova self-requested a review December 9, 2024 20:06
@vuvova
Copy link
Member

vuvova commented Dec 9, 2024

as @grooverdan wrote, we need to understand why the check isn't working before we decide to remove or ignore it.
Here's how you can find it out:

  • run cmake with whatever options you normally do it
  • then run cmake -USIGNAL_RETURN_TYPE_IS_VOID --debug-trycompile
  • then cd CMakeFiles/CMakeScratch/TryCompile-* && make

the last line might look differently depending on your cmake version and your make tool

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
External Contribution All PRs from entities outside of MariaDB Foundation, Corporation, Codership agreements.
Development

Successfully merging this pull request may close these issues.

6 participants