search

FreeRTOS / FreeRTOS-Plus-TCP

FreeRTOS-Plus-TCP library repository. +TCP files only. Submoduled into https://github.com/FreeRTOS/FreeRTOS and various other repos.
MIT License
125 stars 149 forks source link

Enabling user control of compiler analysis for non ISO statements #1140

Closed thirtytwobits closed 2 months ago

thirtytwobits commented 2 months ago

Description

This change replaces the GCC-specific fix for -pedantic in FreeRTOS_Sockets.c with a configurable solution. The change has no functional impact.

Test Steps

Compiles with GCC, -pedantic, and -Werror

Checklist:

Related Issue

(none)

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

Conflicts:

source/FreeRTOS_Sockets.c

Description

Test Steps

Checklist:

Related Issue

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

thirtytwobits commented 2 months ago

Forgot semicolons. Will update sometime today.

tony-josi-aws commented 2 months ago

@thirtytwobits

Thanks for this PR. As we discussed in #1135, what is your thought on using memcpy to avoid the warning instead of adding macros that translate to compiler specific code in the common source code?

thirtytwobits commented 2 months ago

memcpy is a pretty heavy hand to get around some macros. For example:

https://godbolt.org/z/WePM9W85f

The macros avoid any trickery in the compiler stage isolating it to the pre-processing stage. My assumption was you'd prefer to write idiomatic and typesafe C and accept some macros as a wart rather than the inverse? Even looking at the difference between my lighter-weight hack and the current code -> https://godbolt.org/z/M1soG7rfh <- you're still generating extra instructions at -O0 (should be identical for any other level of optimization) which does show this is actively thwarting the type system for every compiler whereas the macros have no effect until/unless the user adds directives specific to their compiler.

But I'll do this either way. It's your project. I'm just visiting ;-)

htibosch commented 2 months ago

@thirtytwobits

Thanks for this PR. As we discussed in #1135, what is your thought on using memcpy to avoid the warning instead of adding macros that translate to compiler specific code in the common source code?

I also thought of using memcpy(), but a problem will still occur when a function pointer is larger than a void pointer.

htibosch commented 2 months ago

memcpy is a pretty heavy hand to get around some macros. For example:

https://godbolt.org/z/WePM9W85f

The socket option is rarely set, and most memcpy()s are optimised to copy 4 or 8 bytes in a few instructions. But as I said, the use of void * is the problem.

thirtytwobits commented 2 months ago

The socket option is rarely set, and most memcpy()s are optimised to copy 4 or 8 bytes in a few instructions. But as I said, the use of void * is the problem.

but you'd add a bl statement; you would modify the runtime to work around a compiler front-end diagnostic. This seems like the wrong bias.