Skip to content

Conversation

@htibosch
Copy link
Contributor

Description

  • FreeRTOS_DNS.c

Use xReturn and not uxReturn when the variable is signed.
Call memset() to clear the contents of a struct.

I have seen applications crash on such a statement:

-    struct xDNSBuffer xReceiveBuffer = { 0 };
+    struct xDNSBuffer xReceiveBuffer;
+    memset( xReceiveBuffer, 0, sizeof( xReceiveBuffer ) );

because the compile can be mistaken about the alignment.

  • FreeRTOS_DNS_Networking.c

Check the return value of FreeRTOS_recvfrom() before assigning it to an unsigned variable:

-    pxReceiveBuffer->uxPayloadLength = FreeRTOS_recvfrom( xDNSSocket, ... );
-    pxReceiveBuffer->uxPayloadSize = pxReceiveBuffer->uxPayloadLength;
+    lResult = FreeRTOS_recvfrom( xDNSSocket, ... );
+    if( lResult > 0 )
+    {
+        pxReceiveBuffer->uxPayloadLength = ( size_t ) lResult;
+        pxReceiveBuffer->uxPayloadSize = pxReceiveBuffer->uxPayloadLength;
+    }

The result of FreeRTOS_recvfrom() must be tested because a negative value indicates an error.

  • FreeRTOS_IP_Utils.c

As I commented here, cast a BaseType_t variable to int when using snprintf():

-( void ) snprintf( pcBuffer, uxLength, "Errno %d", xErrnum );
+( void ) snprintf( pcBuffer, uxLength, "Errno %d", ( int ) xErrnum );
  • FreeRTOS_IP.h

Let ipFR_TCP_VERSION_NUMBER depend on the other 3 version macros:

-    #define ipFR_TCP_VERSION_NUMBER    "V2.3.999"
     #define ipFR_TCP_VERSION_MAJOR     2
     #define ipFR_TCP_VERSION_MINOR     3
     /* Development builds are always version 999. */
     #define ipFR_TCP_VERSION_BUILD     999
+    #define __VERSION( v1, v2, v3 )    "V" # v1 "." # v2 "." # v3
+    #define _VERSION( v1, v2, v3 )     __VERSION( v1, v2, v3 )
+
+    /* ipFR_TCP_VERSION_NUMBER makes a string like "V2.3.999". */
+    #define ipFR_TCP_VERSION_NUMBER \
+        _VERSION( ipFR_TCP_VERSION_MAJOR, ipFR_TCP_VERSION_MINOR, ipFR_TCP_VERSION_BUILD )

but I think that Aniruddha has a more elegant solution.

I compare today's mainline with the release on 7th of June. Beside a few minor things I think it is all good. Thanks.

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

@htibosch htibosch requested a review from a team as a code owner July 30, 2022 07:27
Copy link
Member

@AniruddhaKanhere AniruddhaKanhere left a comment

Choose a reason for hiding this comment

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

Thank you for this PR @htibosch. I have a couple of comments/suggestions. Can you please take a look at that?

Comment on lines 51 to 52
#define __VERSION( v1, v2, v3 ) "V" # v1 "." # v2 "." # v3
#define _VERSION( v1, v2, v3 ) __VERSION( v1, v2, v3 )
Copy link
Member

Choose a reason for hiding this comment

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

MISRA doesn't like the use of # operator. Ref: Misra c-2012 rule 20.10 (advisory).

Also, these version numbers are updated together - and only when a release happens. Not completely sold on the value added by using the above expression to generate the ipFR_TCP_VERSION_NUMBER string.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I have cancelled the change to the macro ipFR_TCP_VERSION_NUMBER.

@ActoryOu
Copy link
Member

ActoryOu commented Jul 19, 2023

The logic added in DNS_ReadReply is not needed. Removed that part and update the branch.

        if( lResult > 0 )
        {
            pxReceiveBuffer->uxPayloadLength = ( size_t ) lResult;
            pxReceiveBuffer->uxPayloadSize = pxReceiveBuffer->uxPayloadLength;
        }
        else
        {
            pxReceiveBuffer->uxPayloadLength = 0U;
            pxReceiveBuffer->uxPayloadSize = 0U;
        }

@ActoryOu ActoryOu merged commit 929a658 into FreeRTOS:main Jul 20, 2023
@htibosch htibosch deleted the IPv4_single_comments_after_MISRA branch September 17, 2025 06:01
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