Skip to content

Fix compiler warnings with optimizations enabled #1218

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 12 commits into from
Jan 29, 2025
Merged

Fix compiler warnings with optimizations enabled #1218

merged 12 commits into from
Jan 29, 2025

Conversation

@tony-josi-aws
Copy link
Member

@tony-josi-aws tony-josi-aws commented Jan 22, 2025 •
edited
Loading

Description

This PR fixes compiler warnings that are generated when compiled with optimizations enabled in GCC and adds CI build checks with optimization settings on.

Thanks, @StefanBalt, for reporting this issue in #1217.

This PR also fixes the false positive warnings emitted by GCC version GNU 13.1.0:

/home/ubuntu/Projects/FreeRTOS-Plus-TCP/source/FreeRTOS_DNS_Callback.c: In function ‘xDNSDoCallback’:
/home/ubuntu/Projects/FreeRTOS-Plus-TCP/build/_deps/freertos_kernel-src/include/./list.h:241:75: error: array subscript ‘ListItem_t {aka const struct xLIST_ITEM}[0]’ is partly outside array bounds of ‘List_t[1]’ {aka ‘struct xLIST[1]’} [-Werror=array-bounds=]
  241 | #define listGET_NEXT( pxListItem )                        ( ( pxListItem )->pxNext )
      |                                                                           ^~
/home/ubuntu/Projects/FreeRTOS-Plus-TCP/source/FreeRTOS_DNS_Callback.c:72:54: note: in expansion of macro ‘listGET_NEXT’
   72 |             for( pxIterator = ( const ListItem_t * ) listGET_NEXT( pxEnd );
      |                                                      ^~~~~~~~~~~~
/home/ubuntu/Projects/FreeRTOS-Plus-TCP/source/FreeRTOS_DNS_Callback.c:45:19: note: at offset 16 into object ‘xCallbackList’ of size 40
   45 |     static List_t xCallbackList;

Test Steps

Tested with DNS

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.

Related Issue

#1217

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 tony-josi-aws requested a review from a team as a code owner January 22, 2025 10:02
@archigup
Copy link
Member

archigup commented Jan 22, 2025 •
edited
Loading

This will create warnings on compilers that always warn on strcpy.

Do we have Clang in CI?

If it doesnt warn on the compilers we we support, should be fine.

@StefanBalt
Copy link
Contributor

StefanBalt commented Jan 23, 2025

This will create warnings on compilers that always warn on strcpy.

@archigup do you think memcpy() would be the better choice?

@tony-josi-aws
Copy link
Member Author

tony-josi-aws commented Jan 27, 2025 •
edited
Loading

@archigup @StefanBalt
Clang seems to be fine with strcpy: https://godbolt.org/z/7b5bW7f73

@tony-josi-aws tony-josi-aws merged commit 527a7c9 into FreeRTOS:main Jan 29, 2025
10 checks passed
@ig15 ig15 mentioned this pull request Jan 30, 2025
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@aggarg aggarg aggarg approved these changes

+1 more reviewer

@kar-rahul-aws kar-rahul-aws kar-rahul-aws approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@tony-josi-aws @archigup @StefanBalt @aggarg @kar-rahul-aws @amazonKamath