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 compilation with -Wpedantic on GCC #1059

Merged
merged 5 commits into from
Jan 10, 2024
Merged

Conversation

Mixaill
Copy link
Contributor

@Mixaill Mixaill commented Dec 7, 2023

Description

This PR does the following

  • fixes empty declaration of 'enum' type does not redeclare tag error in the FreeRTOS_IPv4.h file.
  • adds -Wpedantic to the GCC compilation flags
    • adds -Wno-pedantic for FreeRTOS_Sockets.c file in case you are using GCC to suppress warning

The main reason is that FreeRTOS_IPv4.h is a public header and in case an application uses -Wpedantic -Werror to compile, it will not be able to use FreeRTOS-Plus-TCP.

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.

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

@Mixaill Mixaill requested a review from a team as a code owner December 7, 2023 15:22
struct xIP_PACKET;

#ifdef __GNUC__
Copy link
Member

@moninom1 moninom1 Dec 14, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this needed, i see that by adding -Wpedantic to the GCC compilation flags only gives error in FreeRTOS_Sockets.c which you resolved above.

Copy link
Contributor Author

@Mixaill Mixaill Dec 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reproduced in GCC 13.2

It looks like -std=gnu2x is also required to reproduce the error, but since it is a public header, this should be handled.

/opt/arm/gcc-13.2/bin/arm-none-eabi-gcc 

-mcpu=cortex-m7 -mfpu=fpv5-d16 -mfloat-abi=hard -mthumb -mlong-calls -mlittle-endian -Wall -Wextra -Werror -Wpedantic -Wno-psabi -fstack-usage -ffunction-sections -fdata-sections -fno-omit-frame-pointer -fvar-tracking -fvar-tracking-assignments -O0 -DDEBUG  -ggdb3 -gdwarf-5 -std=gnu2x -fdiagnostics-color=always -Wno-array-bounds -Wno-strict-aliasing -Wno-unused-but-set-variable

...
FreeRTOS_IPv4.h:46:10: error: empty declaration of 'enum' type does not redeclare tag [-Werror=pedantic]
   46 |     enum eFrameProcessingResult;

source/FreeRTOS_Sockets.c Outdated Show resolved Hide resolved
@moninom1
Copy link
Member

Hi @Mixaill

Can you please help in updating on the above review comments.

@Mixaill
Copy link
Contributor Author

Mixaill commented Jan 5, 2024

@moninom1 Please take a look once again

moninom1
moninom1 previously approved these changes Jan 9, 2024
tony-josi-aws
tony-josi-aws previously approved these changes Jan 9, 2024
@tony-josi-aws tony-josi-aws dismissed their stale review January 10, 2024 08:02

Dismissing review as we do not want to add compiler specific stuff in the common source files

@aggarg
Copy link
Member

aggarg commented Jan 10, 2024

Thank you for your contribution. I have the following suggestions:

  • Warning from source/FreeRTOS_Sockets.c - We do not want to add compiler specific stuff in the common source files as those are built with multiple compilers. The recommended way to suppress the warning is to suppress it in the build system (preferably only for this file) if you enable pedantic option.

  • Warning from source/include/FreeRTOS_IPv4.h - The correct way to address this warning is to remove the forward declaration of eFrameProcessingResult and move the following 2 function declarations to source/include/FreeRTOS_IPv4_Private.h:

    /* The function 'prvAllowIPPacket()' checks if a packets should be processed. */
    enum eFrameProcessingResult prvAllowIPPacketIPv4( const struct xIP_PACKET * const pxIPPacket,
                                                      const struct xNETWORK_BUFFER * const pxNetworkBuffer,
                                                      UBaseType_t uxHeaderLength );
    
    /* Check if the IP-header is carrying options. */
    enum eFrameProcessingResult prvCheckIP4HeaderOptions( struct xNETWORK_BUFFER * const pxNetworkBuffer );
    

    @tony-josi-aws has verified that these changes fix the warning - 129b4ce.

Would you like to make these changes or would you like us to?

@Mixaill
Copy link
Contributor Author

Mixaill commented Jan 10, 2024

@aggarg

  • replaced changes in FreeRTOS_Sockets.c with -Wno-pedantic for this file
  • replaced changes in FreeRTOS_IPv4.h with commits from @tony-josi-aws

@tony-josi-aws tony-josi-aws merged commit bedefe0 into FreeRTOS:main Jan 10, 2024
10 checks passed
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.

4 participants