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.

Sign up for GitHub

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

Correct GCC warnings #798

Merged
merged 3 commits into from
Mar 21, 2023
Merged

Correct GCC warnings #798

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
4a3527b
Correct GCC warnings
Mar 15, 2023
45902f1
PR feedback
kstribrnAmzn Mar 17, 2023
3bbf3ee
Merge branch 'main' into warningsFixGCC
amazonKamath Mar 21, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 1 addition & 6 deletions CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -161,11 +161,6 @@ else()
endif()
endif()

########################################################################
# Requirements
set(CMAKE_C_STANDARD 90) # Note FreeRTOS-Kernel uses C99 constructs.
set(CMAKE_C_STANDARD_REQUIRED ON)

########################################################################
# Overall Compile Options
# Note the compile option strategy is to error on everything and then
Expand Down Expand Up @@ -195,9 +190,9 @@ add_compile_options(

$<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wall>
$<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wextra>
$<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wpedantic>
$<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Werror>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Weverything>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wpedantic>

# TODO: Add in other Compilers here.
)
Expand Down
7 changes: 3 additions & 4 deletions source/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
add_library( freertos_plus_tcp STATIC )

set_property(TARGET freertos_plus_tcp PROPERTY C_STANDARD 90)

target_sources( freertos_plus_tcp
PRIVATE
include/FreeRTOS_ARP.h
Expand Down Expand Up @@ -74,10 +76,7 @@ target_compile_options( freertos_plus_tcp
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-shorten-64-to-32>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-sign-conversion>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-macros>
$<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-unused-but-set-variable>
$<$<COMPILE_LANG_AND_ID:C,Clang,GNU>:-Wno-unused-parameter>
$<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-unused-variable>
AniruddhaKanhere marked this conversation as resolved.
Show resolved Hide resolved
$<$<COMPILE_LANG_AND_ID:C,GNU>:-Wno-pedantic>
$<$<COMPILE_LANG_AND_ID:C,Clang>:-Wno-unused-parameter>
)

target_link_libraries( freertos_plus_tcp
Expand Down
2 changes: 1 addition & 1 deletion source/FreeRTOS_IP.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1002,10 +1002,10 @@ void FreeRTOS_ReleaseUDPPayloadBuffer( void const * pvBuffer )
static uint16_t usSequenceNumber = 0;
uint8_t * pucChar;
size_t uxTotalLength;
BaseType_t xEnoughSpace;
IPStackEvent_t xStackTxEvent = { eStackTxEvent, NULL };

uxTotalLength = uxNumberOfBytesToSend + sizeof( ICMPPacket_t );
BaseType_t xEnoughSpace;

if( uxNumberOfBytesToSend < ( ipconfigNETWORK_MTU - ( sizeof( IPHeader_t ) + sizeof( ICMPHeader_t ) ) ) )
{
Expand Down
34 changes: 21 additions & 13 deletions source/FreeRTOS_TCP_IP.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -634,28 +634,36 @@
/* Function might modify the parameter. */
NetworkBufferDescriptor_t * pxNetworkBuffer = pxDescriptor;

configASSERT( pxNetworkBuffer != NULL );
configASSERT( pxNetworkBuffer->pucEthernetBuffer != NULL );

/* Map the buffer onto a ProtocolHeaders_t struct for easy access to the fields. */

/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
const ProtocolHeaders_t * pxProtocolHeaders = ( ( const ProtocolHeaders_t * )
&( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER + xIPHeaderSize( pxNetworkBuffer ) ] ) );
ProtocolHeaders_t * pxProtocolHeaders;
FreeRTOS_Socket_t * pxSocket;
uint16_t ucTCPFlags = pxProtocolHeaders->xTCPHeader.ucTCPFlags;
uint16_t ucTCPFlags;
uint32_t ulLocalIP;
uint16_t usLocalPort = FreeRTOS_htons( pxProtocolHeaders->xTCPHeader.usDestinationPort );
uint16_t usRemotePort = FreeRTOS_htons( pxProtocolHeaders->xTCPHeader.usSourcePort );
uint16_t usLocalPort;
uint16_t usRemotePort;
uint32_t ulRemoteIP;
uint32_t ulSequenceNumber = FreeRTOS_ntohl( pxProtocolHeaders->xTCPHeader.ulSequenceNumber );
uint32_t ulAckNumber = FreeRTOS_ntohl( pxProtocolHeaders->xTCPHeader.ulAckNr );
uint32_t ulSequenceNumber;
uint32_t ulAckNumber;
BaseType_t xResult = pdPASS;

const IPHeader_t * pxIPHeader;

configASSERT( pxNetworkBuffer != NULL );
configASSERT( pxNetworkBuffer->pucEthernetBuffer != NULL );

/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
pxProtocolHeaders = ( ( ProtocolHeaders_t * )
&( pxNetworkBuffer->pucEthernetBuffer[ ipSIZE_OF_ETH_HEADER + xIPHeaderSize( pxNetworkBuffer ) ] ) );

ucTCPFlags = pxProtocolHeaders->xTCPHeader.ucTCPFlags;
usLocalPort = FreeRTOS_htons( pxProtocolHeaders->xTCPHeader.usDestinationPort );
usRemotePort = FreeRTOS_htons( pxProtocolHeaders->xTCPHeader.usSourcePort );
ulSequenceNumber = FreeRTOS_ntohl( pxProtocolHeaders->xTCPHeader.ulSequenceNumber );
ulAckNumber = FreeRTOS_ntohl( pxProtocolHeaders->xTCPHeader.ulAckNr );

/* Check for a minimum packet size. */
if( pxNetworkBuffer->xDataLength < ( ipSIZE_OF_ETH_HEADER + xIPHeaderSize( pxNetworkBuffer ) + ipSIZE_OF_TCP_HEADER ) )
{
Expand Down
3 changes: 2 additions & 1 deletion source/FreeRTOS_UDP_IP.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -310,6 +310,7 @@ BaseType_t xProcessReceivedUDPPacket( NetworkBufferDescriptor_t * pxNetworkBuffe
{
BaseType_t xReturn = pdPASS;
FreeRTOS_Socket_t * pxSocket;
const UDPPacket_t * pxUDPPacket;

configASSERT( pxNetworkBuffer != NULL );
configASSERT( pxNetworkBuffer->pucEthernetBuffer != NULL );
Expand All @@ -319,7 +320,7 @@ BaseType_t xProcessReceivedUDPPacket( NetworkBufferDescriptor_t * pxNetworkBuffe
/* MISRA Ref 11.3.1 [Misaligned access] */
/* More details at: https://github.com/FreeRTOS/FreeRTOS-Plus-TCP/blob/main/MISRA.md#rule-113 */
/* coverity[misra_c_2012_rule_11_3_violation] */
const UDPPacket_t * pxUDPPacket = ( ( const UDPPacket_t * ) pxNetworkBuffer->pucEthernetBuffer );
pxUDPPacket = ( ( UDPPacket_t * ) pxNetworkBuffer->pucEthernetBuffer );

/* Caller must check for minimum packet size. */
pxSocket = pxUDPSocketLookup( usPort );
Expand Down
3 changes: 1 addition & 2 deletions source/portable/BufferManagement/BufferAllocation_2.c
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -73,11 +73,10 @@
#define ASSERT_CONCAT_( a, b ) a ## b
#define ASSERT_CONCAT( a, b ) ASSERT_CONCAT_( a, b )
#define STATIC_ASSERT( e ) \
; enum { ASSERT_CONCAT( assert_line_, __LINE__ ) = 1 / ( !!( e ) ) }
enum { ASSERT_CONCAT( assert_line_, __LINE__ ) = 1 / ( !!( e ) ) }
Copy link
Member

Choose a reason for hiding this comment

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

I assume this change did not break anything.
I wonder if this was done to please a specific compiler...
Do you know the reason @htibosch?

Copy link
Contributor

Choose a reason for hiding this comment

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

@AniruddhaKanhere wrote:

I assume this change did not break anything.
I wonder if this was done to please a specific compiler...
Do you know the reason @htibosch?

The semicolon isn't necessary. It doesn't change the behaviour.

As you see this is a static assert of the expression ipconfigETHERNET_MINIMUM_PACKET_BYTES <= baMINIMAL_BUFFER_SIZE, where baMINIMAL_BUFFER_SIZE uses the sizeof operator.

I just tested these static asserts:

    STATIC_ASSERT( 5 > 4 );
    STATIC_ASSERT( 4 > 4 );  /* error */
    STATIC_ASSERT( 2 > 1 );
    STATIC_ASSERT( 4 > 5 );  /* error */

The compiler listed 2 failures:

    enumerator value for 'assert_line_87' is not an integer constant
	enumerator value for 'assert_line_89' is not an integer constant

ipconfigETHERNET_MINIMUM_PACKET_BYTES is the minimum number of bytes in a packet on the wire. When necessary, the packet is extended with zero's in order to get the minimum length.

baMINIMAL_BUFFER_SIZE is the minimum number of bytes that are allocated in a network buffer.

In the code, it is assumed that a network buffer always has enough space to store ipconfigETHERNET_MINIMUM_PACKET_BYTES bytes.

Copy link

Choose a reason for hiding this comment

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

Just for documentation purposes: TI ARM CGT also complained about the semicolon.


STATIC_ASSERT( ipconfigETHERNET_MINIMUM_PACKET_BYTES <= baMINIMAL_BUFFER_SIZE );
#endif

/* A list of free (available) NetworkBufferDescriptor_t structures. */
static List_t xFreeBuffersList;

Expand Down
11 changes: 8 additions & 3 deletions test/build-combination/Common/FreeRTOSConfig.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -116,13 +116,18 @@

/* The function that implements FreeRTOS printf style output, and the macro
* that maps the configPRINTF() macros to that function. */
#define configPRINTF( X )
void vLoggingPrintf( char const * pcFormat,
... );

/* The function that implements FreeRTOS printf style output, and the macro
* that maps the configPRINTF() macros to that function. */
#define configPRINTF( X ) vLoggingPrintf X

/* Non-format version thread-safe print. */
#define configPRINT( X )
#define configPRINT( X ) vLoggingPrintf X

/* Non-format version thread-safe print. */
#define configPRINT_STRING( X )
#define configPRINT_STRING( X ) vLoggingPrintf X

/* Application specific definitions follow. **********************************/

Expand Down