Skip to content

Fix a few minor compiler/standards issues (clang-16 porting) (0.103.9)#747

Merged
val-ms merged 2 commits intoCisco-Talos:dev/0.103.9from
orlitzky:clang-16
May 23, 2023
Merged

Fix a few minor compiler/standards issues (clang-16 porting) (0.103.9)#747
val-ms merged 2 commits intoCisco-Talos:dev/0.103.9from
orlitzky:clang-16

Conversation

@orlitzky
Copy link
Contributor

@orlitzky orlitzky commented Nov 5, 2022

The next major version of clang has threatened to enable some strictness flags by default:

https://bugs.gentoo.org/show_bug.cgi?id=clang-16-porting

Specifically,

  • -Werror=implicit-function-declaration
  • -Werror=implicit-int
  • -Werror=strict-prototypes

Whether or not they all wind up on by default, the problems they catch are easy to fix and usually beneficial for e.g. further compiler diagnostics. Here I've fixed what I had to in order to build v0.103.7.

For the autotools build system, the following autoconf fix (and an autoreconf -fi) is also required:

https://git.savannah.gnu.org/gitweb/?p=autoconf.git;a=commit;h=8b5e2016c7ed2d67f31b03a3d2e361858ff5299b

Otherwise, the checks that are emitted will unduly fail.

@val-ms val-ms changed the base branch from rel/0.103 to dev/0.103.8 November 5, 2022 01:50
@val-ms val-ms added this to the 0.103.8 milestone Nov 5, 2022
@val-ms
Copy link
Contributor

val-ms commented Nov 5, 2022

Thanks for submitting this. Skimming over the changes it all looks good to me.
I imagine this all applies to the main branch as well.

We don't commit new development directly to our release branches, so I've created dev/0.103.8 and changed this PR to target it instead.

I also don't like to merge PR's into a development branch until we've changed the clam version number in that branch, so even once this is tested, we'll hold off until that is done.

@orlitzky
Copy link
Contributor Author

orlitzky commented Nov 5, 2022

Thanks for submitting this. Skimming over the changes it all looks good to me. I imagine this all applies to the main branch as well.

Yes, probably. We have two similar bugs reported against v0.105.x (871252, 879625), but I'm only maintaining the LTS these days. If you're feeling proactive, you can try to build the main branch yourself with those three -Werror=... flags enabled. They work just as well in GCC.

We don't commit new development directly to our release branches, so I've created dev/0.103.8 and changed this PR to target it instead.

I also don't like to merge PR's into a development branch until we've changed the clam version number in that branch, so even once this is tested, we'll hold off until that is done.

No problem. Thanks.

@val-ms val-ms changed the title Fix a few minor compiler/standards issues (clang-16 porting) Fix a few minor compiler/standards issues (clang-16 porting) (0.103.8) Nov 23, 2022
@val-ms
Copy link
Contributor

val-ms commented Mar 7, 2023

Ugh. We didn't merge this before releasing 0.103.8. I'm sorry. We can try to put this towards 0.103.9. At least we did merge the one that went into main towards future versions.

@val-ms val-ms changed the title Fix a few minor compiler/standards issues (clang-16 porting) (0.103.8) Fix a few minor compiler/standards issues (clang-16 porting) (0.103.9) Mar 7, 2023
@val-ms val-ms modified the milestones: 0.103.8, 0.103.9 Mar 7, 2023
@orlitzky
Copy link
Contributor Author

orlitzky commented Mar 8, 2023

NBD. Clang 16 is expected "any day now" so I was peer pressured into patching v0.103.8. But with the LTS series on life-support-only the patches aren't that hard to manage.

@val-ms val-ms changed the base branch from dev/0.103.8 to dev/0.103.9 March 9, 2023 03:44
orlitzky added 2 commits May 19, 2023 15:59
On Linux, thpool.c uses syscall() from unistd.h, but that function is
not defined without _GNU_SOURCE:

  c-thread-pool/thpool.c: In function 'jobqueue_pull':
  c-thread-pool/thpool.c:474:105: error: implicit declaration of function
  'syscall' [-Werror=implicit-function-declaration]

In general that's not great, because it hinders some compiler diagnostics,
but it will also cause problems down the road if (for example) clang-16
decides to enable -Werror=implicit-function-declaration by default.

This commit changes the _POSIX_C_SOURCE definition at the top of
thpool.c to _GNU_SOURCE, as in the syscall(2) man page.
Prototypes (or the declarations themselves, if there is no
corresponding prototype) for functions that take no arguments are
required by the C standard to specify (void) as their argument list;
for example,

  regex_pcre.h:79:1: error: function declaration isn't a prototype
  [-Werror=strict-prototypes]
     79 | cl_error_t cli_pcre_init_internal();

Future versions of clang may become strict about this, and there's no
harm in conforming to the standard right now, so we fix all such
instances in this commit.
@val-ms val-ms merged commit d0179cc into Cisco-Talos:dev/0.103.9 May 23, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants