Enabling user control of compiler analysis for non ISO statements

[thirtytwobits]

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:

• I have tested my changes. No regression in existing tests.
• I have modified and/or added unit-tests to cover the code changes in this Pull Request.
• I have tested by changes but have not run the existing tests.

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:

• I have tested my changes. No regression in existing tests.
• I have modified and/or added unit-tests to cover the code changes in this Pull Request.

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.

[jasonpcarroll]

Looks good! I will get someone else to also take a look. You will need to update the formatting per the CI check, but that can easily be done by applying the git patch found under Artifacts->formattingChanges on the summary page of the CI run.

[thirtytwobits]

Forgot semicolons. Will update sometime today.

[tony-josi-aws]

@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]

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]

@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]

The following assignment seems to be a mortal sin:

    typedef void (* SocketWakeupCallback_t )( Socket_t * pxSocket );
    SocketWakeupCallback_t pxUserWakeCallback;

    pxUserWakeCallback = ( SocketWakeupCallback_t ) pvOptionValue;

The problem is that we treat a function pointer as if it were a void pointer. The reason for this is that FreeRTOS_setsockopt() uses a void parameter:

    BaseType_t FreeRTOS_setsockopt( Socket_t xSocket,
                                    int32_t lLevel,
                                    int32_t lOptionName,
                                    const void * pvOptionValue,
                                    size_t uxOptionLength );

The option FREERTOS_SO_WAKEUP_CALLBACK will bind a user function to a socket.
On some platforms, the two pointers (void* versus function*) should be treated in a different way.

Thanks for this PR, I approve it.

[htibosch]

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]

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.