Skip to content

Fix prvWriteMessageToBuffer on big endian #391

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

Jump to bottom
Merged
merged 4 commits into from
Oct 16, 2021
Merged

Fix prvWriteMessageToBuffer on big endian #391

merged 4 commits into from
Oct 16, 2021

Conversation

archigup
Copy link
Member

prvWriteMessageToBuffer wrote the first sbBYTES_TO_STORE_MESSAGE_LENGTH
bytes of the size_t-typed length to the buffer as the data length. While
this functions on little endian, it copies the wrong bytes on big
endian. This fix converts the length to configMESSAGE_BUFFER_LENGTH_TYPE
first, and then copies the exact amount, thus fixing the issue.
Additionally it adds an assert to verify the size is not greater than
the max value of configMESSAGE_BUFFER_LENGTH_TYPE; previously this would
truncate silently.

Fixes #389.

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

@archigup archigup requested a review from a team as a code owner September 11, 2021 02:37
@codecov
Copy link

codecov bot commented Sep 11, 2021 •
edited
Loading

Codecov Report

Merging #391 (e04048c) into main (68ddb32) will increase coverage by 0.01%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main     #391      +/-   ##
==========================================
+ Coverage   92.13%   92.15%   +0.01%     
==========================================
  Files           4        4              
  Lines        1272     1274       +2     
  Branches      342      343       +1     
==========================================
+ Hits         1172     1174       +2     
  Misses         53       53              
  Partials       47       47
Flag Coverage Δ
unittests 92.15% <100.00%> (+0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
stream_buffer.c 96.15% <100.00%> (+0.02%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 06fb777...e04048c. Read the comment docs.

yourslab
yourslab reviewed Sep 23, 2021
View reviewed changes
@archigup
Fix prvWriteMessageToBuffer on big endian
ae4ff36 
prvWriteMessageToBuffer wrote the first sbBYTES_TO_STORE_MESSAGE_LENGTH
bytes of the size_t-typed length to the buffer as the data length. While
this functions on little endian, it copies the wrong bytes on big
endian. This fix converts the length to configMESSAGE_BUFFER_LENGTH_TYPE
first, and then copies the exact amount, thus fixing the issue.
Additionally it adds an assert to verify the size is not greater than
the max value of configMESSAGE_BUFFER_LENGTH_TYPE; previously this would
truncate silently.
@dry-75
Copy link

dry-75 commented Oct 14, 2021

So this died here, or was the fix merged into .. something ?

@archigup
Copy link
Member Author

Unit tests need to be updated to hit the assert, so that this can be merged

@alfred2g alfred2g mentioned this pull request Oct 15, 2021
Merged
@alfred2g
Copy link
Contributor

/bot run checks

@aggarg aggarg merged commit d649a77 into FreeRTOS:main Oct 16, 2021
@archigup archigup deleted the big_endian_fix branch October 18, 2021 06:06
dawood87 added a commit to dawood87/FreeRTOS-Kernel that referenced this pull request Oct 19, 2021
@dawood87
Squashed commit of the following:
122cffa 
commit 6ab51bb
Author: dawood87 <[email protected]>
Date:   Tue Oct 19 14:32:34 2021 +0200

    Squashed commit of the following:

    commit d649a77
    Author: Archit Gupta <[email protected]>
    Date:   Sat Oct 16 16:36:44 2021 -0700

        Fix prvWriteMessageToBuffer on big endian (FreeRTOS#391)

        prvWriteMessageToBuffer wrote the first sbBYTES_TO_STORE_MESSAGE_LENGTH
        bytes of the size_t-typed length to the buffer as the data length. While
        this functions on little endian, it copies the wrong bytes on big
        endian. This fix converts the length to configMESSAGE_BUFFER_LENGTH_TYPE
        first, and then copies the exact amount, thus fixing the issue.
        Additionally it adds an assert to verify the size is not greater than
        the max value of configMESSAGE_BUFFER_LENGTH_TYPE; previously this would
        truncate silently.

        Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

    commit 06fb777
    Author: Gaurav-Aggarwal-AWS <[email protected]>
    Date:   Sat Oct 16 13:45:03 2021 -0700

        Update comments for the ARM_CA53_64_BIT_SRE port (FreeRTOS#403)

        Mention that FreeRTOS_IRQ_Handler should not be used for FIQs and the
        reason for assuming Group 1 for Interrupt Acknowledge and End Of
        Interrupt registers.

        Signed-off-by: Gaurav Aggarwal <[email protected]>

    commit 68ddb32
    Author: Stephane Viau <[email protected]>
    Date:   Fri Oct 15 18:21:56 2021 +0200

        Handle interrupt acknowledge register in Cortex-A53 SRE port (FreeRTOS#392)

        Let the FreeRTOS IRQ handler properly store and restore the ICCIAR
        register value around the vApplicationIRQHandler() call.

        Signed-off-by: Stephane Viau <[email protected]>

        Co-authored-by: Stephane Viau <[email protected]>
        Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

    commit a030d0a
    Author: swaldhoer <[email protected]>
    Date:   Thu Oct 7 00:32:07 2021 +0200

        fix typo (FreeRTOS#399)

commit 226f4b9
Author: dawood87 <[email protected]>
Date:   Tue Oct 19 14:32:04 2021 +0200

    Squashed commit of the following:

    commit 5f274a2
    Merge: 14fddf1 763f1b8
    Author: dawood87 <[email protected]>
    Date:   Mon Oct 18 17:37:46 2021 +0200

        Merge branch '347-omit-size-in-extern-uint8t-ucHeap' of https://github.com/starcopter/FreeRTOS-Kernel into pr/351

    commit 14fddf1
    Author: dawood87 <[email protected]>
    Date:   Fri Oct 15 13:38:24 2021 +0200

        Squashed commit of the following:

        commit a030d0a
        Author: swaldhoer <[email protected]>
        Date:   Thu Oct 7 00:32:07 2021 +0200

            fix typo (FreeRTOS#399)

        commit 1fb4e84
        Author: Qikai <[email protected]>
        Date:   Tue Oct 5 02:15:00 2021 +0800

            Fix the defect that Heap_1.c may waste first portBYTE_ALIGNMENT bytes of ucHeap[] (FreeRTOS#238)

            * Fix the defect that Heap_1.c may waste first 8 bytes of ucHeap[]

            * Fix the same byte waste issue in heap_2

        commit 5f290e4
        Author: Andres O. Vela <[email protected]>
        Date:   Thu Sep 30 19:18:47 2021 +0200

            Fix typo in comment (FreeRTOS#398)

        commit 741185f
        Author: Shubham Kulkarni <[email protected]>
        Date:   Fri Sep 17 02:46:22 2021 +0530

            Xtensa_ESP32: Add definition for portMEMORY_BARRIER (FreeRTOS#395)

            This fixes crash observed in Amazon FreeRTOS when optimisations are enabled

        commit 1b86b39
        Author: Gaurav-Aggarwal-AWS <[email protected]>
        Date:   Tue Sep 14 19:25:46 2021 -0700

            Remove AVR ports from main repo (FreeRTOS#394)

            These ports now exist in the
            https://github.com/FreeRTOS/FreeRTOS-Kernel-Partner-Supported-Ports repo.

            Signed-off-by: Gaurav Aggarwal <[email protected]>

        commit 384ffc5
        Author: Gaurav Aggarwal <[email protected]>
        Date:   Fri Sep 10 23:27:12 2021 +0000

            Fix spell-check failure

            Signed-off-by: Gaurav Aggarwal <[email protected]>

        commit 68889fd
        Author: Gaurav Aggarwal <[email protected]>
        Date:   Fri Sep 10 23:20:36 2021 +0000

            Update History.txt

            Signed-off-by: Gaurav Aggarwal <[email protected]>

        commit 99a5a5f
        Author: Gaurav Aggarwal <[email protected]>
        Date:   Tue Aug 10 00:00:35 2021 -0700

            Fix free secure context for Cortex-M23 ports

            Update the branching condition to correctly free secure context when
            there is one.

            Signed-off-by: Gaurav Aggarwal <[email protected]>

        commit 06ea727
        Author: Gaurav Aggarwal <[email protected]>
        Date:   Wed Aug 4 15:00:47 2021 -0700

            Implement secure stack sealing as per ARM's recommendation

            Signed-off-by: Gaurav Aggarwal <[email protected]>

        commit 61f7560
        Author: Gaurav Aggarwal <[email protected]>
        Date:   Wed Aug 4 14:57:45 2021 -0700

            Associate secure context with task handle

            The secure side context management code now checks that the secure
            context being saved or restored belongs to the task being switched-out
            or switched-in respectively.

            Signed-off-by: Gaurav Aggarwal <[email protected]>

        commit ccaa0f4
        Author: Gaurav Aggarwal <[email protected]>
        Date:   Wed Aug 4 14:52:22 2021 -0700

            Pre-allocate secure-side context structures

            This commit improves ARMv8-M security by pre-allocating secure-side task
            context structures and changing how tasks reference a secure-side
            context structure when calling a secure function. The new configuration
            constant secureconfigMAX_SECURE_CONTEXTS sets the number of secure
            context structures to pre-allocate. secureconfigMAX_SECURE_CONTEXTS
            defaults to 8 if left undefined.

            Signed-off-by: Gaurav Aggarwal <[email protected]>

        commit f8ada39
        Author: Zim Kalinowski <[email protected]>
        Date:   Wed Sep 8 03:03:12 2021 +0800

            Replace <pre> with @code - remaining files (FreeRTOS#388)

            Co-authored-by: Paul Bartell <[email protected]>
            Co-authored-by: Ming Yue <[email protected]>

        commit fa0f5c4
        Author: Gaurav-Aggarwal-AWS <[email protected]>
        Date:   Wed Sep 1 15:38:07 2021 -0700

            Add freertos_risc_v_chip_specific_extensions.h for 64-bit ports (FreeRTOS#385)

            * Add freertos_risc_v_chip_specific_extensions.h for 64-bit ports

            Signed-off-by: Gaurav Aggarwal <[email protected]>

        commit bb02cf6
        Author: Zim Kalinowski <[email protected]>
        Date:   Wed Sep 1 02:24:56 2021 +0800

            minor fix in stream buffer doc (FreeRTOS#387)

            Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

        commit ae73f0d
        Author: Zim Kalinowski <[email protected]>
        Date:   Wed Sep 1 00:04:36 2021 +0800

            Replace <pre> with @code{c} (FreeRTOS#386)

            * replace <pre> with @code{c}

            * endcode must pass spellcheck

        commit c290780
        Author: sherryzhang <[email protected]>
        Date:   Fri Aug 27 14:07:58 2021 +0800

            Update the README to align with TF-Mv1.4.0 in TF-M integration (FreeRTOS#384)

            Change-Id: I41fc8e18657086e86eacd38ed70f474555739a3c
            Signed-off-by: Sherry Zhang <[email protected]>

        commit 0b1e9d7
        Author: Zim Kalinowski <[email protected]>
        Date:   Fri Aug 13 09:15:57 2021 +0800

            fixes in queue documentation (FreeRTOS#382)

            Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

        commit 6ba8aa6
        Author: Shubham Kulkarni <[email protected]>
        Date:   Fri Aug 13 06:16:25 2021 +0530

            Xtensa_ESP32: Fix build issues when external SPIRAM is enabled (FreeRTOS#381)

            Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

        commit d858d1f
        Author: Paul Adelsbach <[email protected]>
        Date:   Thu Aug 12 17:06:41 2021 -0700

            fix example usage of xMessageBufferCreateStatic and xStreamBufferCrea… (FreeRTOS#380)

            Example usage is actually correct, so remove the -1. but update
            the incorrect parameter description for pucStreamBufferStorageArea
            and pucMessageBufferStorageArea.

        commit d018018
        Author: Jack Lam <[email protected]>
        Date:   Fri Aug 13 02:50:52 2021 +0800

            Tidy up the 8051 sdcc port (FreeRTOS#376)

            * Tidy up the 8051 sdcc port

            * Replace tabs with spaces in SDCC Cygnal port.c file.

            Co-authored-by: John Lin <[email protected]>
            Co-authored-by: Paul Bartell <[email protected]>

        commit 1b38078
        Author: Zim Kalinowski <[email protected]>
        Date:   Fri Aug 13 02:18:53 2021 +0800

            fixed parameter names documentation (FreeRTOS#378)

        commit b97bb48
        Author: RichardBarry <[email protected]>
        Date:   Wed Aug 4 10:05:23 2021 -0700

            Indent contents of a taskENTER_CRITICAL/taskEXIT_CRITICAL block. (FreeRTOS#348)

            * Indent contents of a taskENTER_CRITICAL/taskEXIT_CRITICAL block.
            Move a few configASSERT() statements out of a path where they would always be triggered to prevent "condition is always true" compiler warnings.

            * Replace configASSERT() positions due to unintended semantic change from the version where asserts were at the top of the file.

            Co-authored-by: RichardBarry <[email protected]>
            Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

        commit ce81bcb
        Author: alfred gedeon <[email protected]>
        Date:   Wed Jul 28 17:53:10 2021 -0700

            Run uncrustify with github workflows (FreeRTOS#369)

            * uncrustify with github workflows

            * Fix find expression

            * Add uncrustify configuration file

            * Uncrustify some files

            * uncrustify some more files

            * uncrustify more files

            * Fix whitespace at end of lines

            Co-authored-by: Cobus van Eeden <[email protected]>

        commit 85a2312
        Author: Gaurav-Aggarwal-AWS <[email protected]>
        Date:   Wed Jul 28 10:37:51 2021 -0700

            Add ReadMe for third party port contributions (FreeRTOS#371)

            * Add ReadMe for third party port contributions

            Signed-off-by: Gaurav Aggarwal <[email protected]>

        commit d9d5d53
        Author: Craig Kewley <[email protected]>
        Date:   Wed Jul 21 01:21:18 2021 +0100

            doc: fix function name typo (FreeRTOS#368)

        commit b5a9229
        Author: Kristine Jassmann <[email protected]>
        Date:   Tue Jul 20 15:55:49 2021 -0400

            Warning fixes. (FreeRTOS#356)

            * Use cast to fix warnings.

            * Remove all empty definitions of portCLEAN_UP_TCB( pxTCB ) and
              portALLOCATE_SECURE_CONTEXT( ulSecureStackSize ) from ports.
              When these are undefined, the default empty definition is defined
              in FreeRTOS.h.

    commit 236a458
    Author: Lasse Fröhner <[email protected]>
    Date:   Fri Jun 18 15:48:24 2021 +0000

        Change ucHeap[] To Pointer Type

    commit 6768094
    Author: Lasse Fröhner <[email protected]>
    Date:   Wed Jun 9 11:32:17 2021 +0000

        Omit configTOTAL_HEAP_SIZE with Application Allocated Heap

        See FreeRTOS#347

    commit 763f1b8
    Merge: f31a1ac 1d86b97
    Author: Lasse Fröhner <[email protected]>
    Date:   Sat Jul 17 13:32:47 2021 +0200

        Merge branch 'main' into 347-omit-size-in-extern-uint8t-ucHeap

    commit f31a1ac
    Author: Lasse Fröhner <[email protected]>
    Date:   Fri Jun 18 15:48:24 2021 +0000

        Change ucHeap[] To Pointer Type

    commit a7eb292
    Merge: 0780328 6a84f2c
    Author: Lasse Fröhner <[email protected]>
    Date:   Sat Jun 12 14:56:18 2021 +0200

        Merge branch 'main' into 347-omit-size-in-extern-uint8t-ucHeap

    commit 0780328
    Merge: d687545 bad8f01
    Author: Lasse Fröhner <[email protected]>
    Date:   Thu Jun 10 23:26:09 2021 +0200

        Merge branch 'main' into 347-omit-size-in-extern-uint8t-ucHeap

    commit d687545
    Author: Lasse Fröhner <[email protected]>
    Date:   Wed Jun 9 11:32:17 2021 +0000

        Omit configTOTAL_HEAP_SIZE with Application Allocated Heap

        See FreeRTOS#347
dawood87 added a commit to dawood87/FreeRTOS-Kernel that referenced this pull request Oct 19, 2021
@dawood87
Squashed commit of the following:
6ab51bb 
commit d649a77
Author: Archit Gupta <[email protected]>
Date:   Sat Oct 16 16:36:44 2021 -0700

    Fix prvWriteMessageToBuffer on big endian (FreeRTOS#391)

    prvWriteMessageToBuffer wrote the first sbBYTES_TO_STORE_MESSAGE_LENGTH
    bytes of the size_t-typed length to the buffer as the data length. While
    this functions on little endian, it copies the wrong bytes on big
    endian. This fix converts the length to configMESSAGE_BUFFER_LENGTH_TYPE
    first, and then copies the exact amount, thus fixing the issue.
    Additionally it adds an assert to verify the size is not greater than
    the max value of configMESSAGE_BUFFER_LENGTH_TYPE; previously this would
    truncate silently.

    Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

commit 06fb777
Author: Gaurav-Aggarwal-AWS <[email protected]>
Date:   Sat Oct 16 13:45:03 2021 -0700

    Update comments for the ARM_CA53_64_BIT_SRE port (FreeRTOS#403)

    Mention that FreeRTOS_IRQ_Handler should not be used for FIQs and the
    reason for assuming Group 1 for Interrupt Acknowledge and End Of
    Interrupt registers.

    Signed-off-by: Gaurav Aggarwal <[email protected]>

commit 68ddb32
Author: Stephane Viau <[email protected]>
Date:   Fri Oct 15 18:21:56 2021 +0200

    Handle interrupt acknowledge register in Cortex-A53 SRE port (FreeRTOS#392)

    Let the FreeRTOS IRQ handler properly store and restore the ICCIAR
    register value around the vApplicationIRQHandler() call.

    Signed-off-by: Stephane Viau <[email protected]>

    Co-authored-by: Stephane Viau <[email protected]>
    Co-authored-by: Gaurav-Aggarwal-AWS <[email protected]>

commit a030d0a
Author: swaldhoer <[email protected]>
Date:   Thu Oct 7 00:32:07 2021 +0200

    fix typo (FreeRTOS#399)
laroche pushed a commit to laroche/FreeRTOS-Kernel that referenced this pull request Apr 18, 2024
@yourslab @abhidixi11
Fix logging issue causing mangled new lines (FreeRTOS#391)
52d7c47 
* Fix logging issue caused by using \n as the default newline character

* Update the other demos

* Fix accidentally changed NETWORK_INTERFACE

* Set \r\n as default instead

* Update sources to reflect \r\n change

* Get rid of configLOGGING_NEW_LINE altogether

Co-authored-by: abhidixi11 <[email protected]>
laroche pushed a commit to laroche/FreeRTOS-Kernel that referenced this pull request Apr 18, 2024
@alfred2g
UT: Add streambuffer test to make PR FreeRTOS#391 pass (FreeRTOS#690)
a32ab34 
UT: Add streambuffer test to make PR FreeRTOS#391 pass
laroche pushed a commit to laroche/FreeRTOS-Kernel that referenced this pull request Apr 18, 2024
@aggarg
Revert "UT: Add streambuffer test to make PR FreeRTOS#391 pass (FreeR…
7340a72 
…TOS#690)"

This reverts commit a32ab34.
laroche pushed a commit to laroche/FreeRTOS-Kernel that referenced this pull request Apr 18, 2024
@aggarg
Revert "Revert "UT: Add streambuffer test to make PR FreeRTOS#391 pass (
a861781 
FreeRTOS#690)""

This reverts commit 7340a72.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@yourslab yourslab yourslab approved these changes

@alfred2g alfred2g alfred2g approved these changes

@dan4thewin dan4thewin dan4thewin approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[BUG] Stream buffer 's configMESSAGE_BUFFER_LENGTH_TYPE is not portable between little / big endian.
6 participants
@archigup @dry-75 @alfred2g @yourslab @dan4thewin @aggarg