Fixing GCC compiler warning

[thirtytwobits]

Fixing GCC compiler warning

Description

This allows compilation of the socket source in projects using GCC with -pedantic= and -Werror. 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.
• [x] 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.

[tony-josi-aws]

@thirtytwobits

Thanks for taking time to contribute to FreeRTOS+TCP. Since, we would like to avoid having compiler specific code in the core library files, selectively disabling the Wpedantic check to the offending line is not possible. The CI checks gets around this problem by disabling the Wpedantic check for the entire FreeRTOS_Sockets.c.

Since the type of pvOptionValue is something that's inherited from the BSD socket, its not trivial or backward compatible to fix it by a type change as well.

[thirtytwobits]

Since, we would like to avoid having compiler specific code in the core library files, selectively disabling the Wpedantic check to the offending line is not possible.

I understand the desire to avoid compiler-specific code however this is inert in all manifestations as it only guides GCC warnings and not actual compiler behaviour and is deactivated entirely for any other compiler. It may be ugly but it is, ultimately, benign.

Since the type of pvOptionValue is something that's inherited from the BSD socket, its not trivial or backward compatible to fix it by a type change as well.

Please consider that this library otherwise compiles with -pedantic. This one line gets in the way of consumers that want to use this check. If you continue to disable -pedantic in your CI more violations will creep in over time and it will be come more and more difficult to change your mind.

Would a version of this that abstracted "GCC" away be more palatable? Something like:

ipconfigISO_STRICTNESS_VIOLATION_START

                        pxSocket->pxUserWakeCallback = ( SocketWakeupCallback_t ) pvOptionValue;

ipconfigISO_STRICTNESS_VIOLATION_END

Users would then be able to redefine these as the necessary pragma push/pop on GCC:

#define ipconfigISO_STRICTNESS_VIOLATION_START _Pragma("GCC diagnostic push") \
_Pragma("GCC diagnostic ignored \"-Wpedantic\"")

#define ipconfigISO_STRICTNESS_VIOLATION_END _Pragma("GCC diagnostic pop")

[tony-josi-aws]

@thirtytwobits

If you continue to disable -pedantic in your CI more violations will creep in over time and it will be come more and more difficult to change your mind.

Yes, disabling the warning for the entire file doesn't solve the problem.

What do you think about the following alternate option:

        const void * pvCopySource = &pvOptionValue;
        void * pvCopyDest = (void *) &( pxSocket->pxUserWakeCallback );
        ( void ) memcpy( pvCopyDest, pvCopySource, sizeof( SocketWakeupCallback_t ) );

instead of pxSocket->pxUserWakeCallback = ( SocketWakeupCallback_t ) pvOptionValue;. This is similar to how Linux handles this.

Would a version of this that abstracted "GCC" away be more palatable? Something like: ipconfigISO_STRICTNESS_VIOLATION_START

Thats more abstract and cleaner when considering multiple compilers. But if possible we would still try to avoid compiler specific stuff in the code.

[tony-josi-aws]

Hi @thirtytwobits, wondering if you prefer to update the PR with proposed changes, if you agree with them, or if you want us to make the changes.

[thirtytwobits]

Yeah, sorry. I owe you these proposed changes. I will get them to you this week.

[tony-josi-aws]

Thanks @thirtytwobits

[thirtytwobits]

Opened a new PR #1140