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

Fix implicit function declarations with clang #382

Merged
merged 3 commits into from
Nov 3, 2023

Conversation

dl1jbe
Copy link
Member

@dl1jbe dl1jbe commented Mar 11, 2023

Will be counted as errors starting with CLANG-16

Will be counted as errors starting with CLANG-16
@zcsahok
Copy link
Member

zcsahok commented Mar 17, 2023

What are the offending functions? Maybe we could just replace them with the "correct" ones instead of adding a magic define.

@dl1jbe
Copy link
Member Author

dl1jbe commented Mar 17, 2023

Sorry, missed to give that information.
It's 'timegm()' in both cases. See man page.

@zcsahok
Copy link
Member

zcsahok commented Mar 18, 2023

I see. Indeed GNU libc manual (https://www.gnu.org/software/libc/manual/html_node/Broken_002ddown-Time.html) says:

timegm is rather rare. For the most portable conversion from a UTC broken-down time to a simple time, set the TZ environment variable to UTC, call mktime, then set TZ back.

We have timegm in log_utils.c and writecabrillo.c too. Couldn't we just set _DEFAULT_SOURCE (and _XOPEN_SOURCE) globally in src/Makefile.am? (When compiling with gcc-10.2.1 both are set automagically.)

@dl1jbe
Copy link
Member Author

dl1jbe commented Mar 20, 2023

We have timegm in log_utils.c and writecabrillo.c too. Couldn't we just set _DEFAULT_SOURCE (and _XOPEN_SOURCE) globally in src/Makefile.am?

Just looked why we get no problems from clang reported there: We have _GNU_SOURCE already defined in that files. 'man feature_test_macros' says about it:

"Since glibc 2.19, defining _GNU_SOURCE also has the effect of implicitly defining _DEFAULT_SOURCE.
Before glibc 2.20, defining _GNU_SOURCE also had the effect of implicitly defining _BSD_SOURCE and _SVID_SOURCE."

One of the problems is that the meaning and use of the macros is slowly changing.

(When compiling with gcc-10.2.1 both are set automagically.)

Unluckily that seems to be handled differently between different compilers.

Wrt adding it to src/Makefile.am. It sure needs some test but i am open for it. But maybe we should add _GNU_SOURCE instead.

@zcsahok
Copy link
Member

zcsahok commented Mar 20, 2023

Defining say _GNU_SOURCE globally would be more manageable in my opinion than various defines scattered around in sources files. Provided of course that this covers all cases.

@dl1jbe
Copy link
Member Author

dl1jbe commented Oct 30, 2023

To bring these to an end. I did some checks for the use of _GNU_SOURCE via Makefile.am. It looks promising. All implicit declarations are gone for actual gcc-13 and also clang 16 and 17.
Will provide an updated PR tomorrow.

@zcsahok
Copy link
Member

zcsahok commented Nov 1, 2023

Thanks!

@dl1jbe dl1jbe merged commit 3e60a4e into Tlf:master Nov 3, 2023
2 checks passed
@dl1jbe dl1jbe deleted the fix_implicit_declarations branch November 3, 2023 08:33
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.

2 participants