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

[BUG] Pointless String Length in prvCreateDNSMessage() Leads to Compiler Warning/Error #1217

Closed
StefanBalt opened this issue Jan 20, 2025 · 3 comments
Labels
bug Something isn't working

Comments

@StefanBalt
Copy link
Contributor

Describe the bug

GCC (with strict settings) raises the following error

error: 'strncpy' specified bound depends on the length of the source argument [-Werror=stringop-overflow=]

for the line below

( void ) strncpy( ( char * ) &( pucUDPPayloadBuffer[ uxIndex ] ), pcHostName, strlen( pcHostName ) + 1U );

and it is not wrong. Why use strncpy() if we do not have a useful length parameter in this scope?

I see 3 possible fixes:

  1. Keep track of the destination buffer size after it is allocated in prvGetPayloadBuffer()
  2. Use strcpy() again (revert change from 4c4223a)
  3. Use memcpy()
@StefanBalt StefanBalt added the bug Something isn't working label Jan 20, 2025
@tony-josi-aws
Copy link
Member

@StefanBalt, thanks for reporting this issue.

This change was done along with other strcpy replacements since its known that pucUDPPayloadBuffer is big enough to hold the pcHostName as its allocated with the size of pcHostName accounted for in the size calculation here.

Since its causing compiler warnings with strict settings, it makes sense to update it, probably back to strcpy or memcpy.

GCC version: gcc (Ubuntu 9.4.0-1ubuntu1~20.04.2) 9.4.0 doesn't produce this warning with any of the -Wstringop-overflow= types [0 to 4]. Are you using any other compiler flags?

@StefanBalt
Copy link
Contributor Author

Any GCC >= 9.2 I tried raised a warning with -Wall -O2 flags. Lower optimization levels do not seem to warn interestingly.

Example in godbolt: https://godbolt.org/z/qaGdMGMKe

@ig15
Copy link
Member

ig15 commented Jan 30, 2025

The issue has been fixed by the PR. Thanks @StefanBalt for bringing it up. Closing the issue.

@ig15 ig15 closed this as completed Jan 30, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

3 participants