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

resolve Wconversion related warnings #935

Closed
wants to merge 1 commit into from

Conversation

timaydin
Copy link

Description

Avoid warnings produced as a result of -Wconversion option to gcc

@moninom1
Copy link
Member

/bot run uncrustify

@moninom1
Copy link
Member

Hi @timaydin

There are some CI checks failure, can you please help in checking them.

Thank You

@AniruddhaKanhere
Copy link
Member

/bot run uncrustify

@@ -331,14 +331,14 @@
*/
if( pucPtr[ 0U ] == tcpTCP_OPT_SACK_A )
{
ucLen -= 2U;
ucLen -= 2;
Copy link
Member

Choose a reason for hiding this comment

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

I am unsure about this change. ucLen is unsigned. Then while assigning 2 to it, why not make 2 explicitly signed by using U?

Copy link
Member

Choose a reason for hiding this comment

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

This will also give MISRA violation.

lIndex += 2;

while( ucLen >= ( uint8_t ) 8U )
{
prvReadSackOption( pucPtr, ( size_t ) lIndex, pxSocket );
lIndex += 8;
ucLen -= 8U;
ucLen -= 8;
Copy link
Member

Choose a reason for hiding this comment

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

This is not solving the issue . Earlier warning was "conversion from ‘unsigned int’ to ‘uint8_t’" with your fix the warning has now changed to conversion from ‘int’ to ‘uint8_t’

@moninom1
Copy link
Member

Hi @timaydin I was reproducing the issue and i can see that there are 40+ warning with enabling Wconverstion. Were you seeing the same number of issue? If not can you let us know how you reproduced the issue?

@moninom1
Copy link
Member

Hi @timaydin, As the issue were way more than fixed in this PR, we have created a new PR. We will be closing this PR for now. Let us know if you need any other info. Once again thank you for bringing this to our attention.

@moninom1 moninom1 closed this Jul 26, 2023
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.

3 participants