-
Notifications
You must be signed in to change notification settings - Fork 64
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
Add sara r4 compile warning check #52
Add sara r4 compile warning check #52
Conversation
chinglee-iot
commented
Nov 4, 2021
•
edited
Loading
edited
- Fix SARA R4 cellular module compile warning to comply with C90
* Add guards for C++ linkage
* Fix sara_r4 compile warning * Add sara_r4 CI compile warning check
modules/sara_r4/cellular_r4.c
Outdated
@@ -490,11 +492,58 @@ uint32_t _Cellular_GetSocketId( CellularContext_t * pContext, | |||
cellularStatus = CELLULAR_BAD_PARAMETER; | |||
} | |||
|
|||
if( ( cellularStatus == CELLULAR_SUCCESS ) && | |||
( sessionId >= MIN_TCP_SESSION_ID ) && ( sessionId <= MAX_TCP_SESSION_ID ) ) | |||
if( ( cellularStatus == CELLULAR_SUCCESS ) && ( sessionId <= MAX_TCP_SESSION_ID ) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The condition ( sessionId >= MIN_TCP_SESSION_ID ) can't be ignored.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
#define MIN_TCP_SESSION_ID ( 0U )
uint32_t _Cellular_GetSocketId( CellularContext_t * pContext,
uint8_t sessionId )
uint8_t is always >= 0
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
But now I see #define MIN_TCP_SESSION_ID ( 0 ) instead of #define MIN_TCP_SESSION_ID ( 0U )
One of them need to be corrected.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for typo, you are right.
#define MIN_TCP_SESSION_ID ( 0 )
./cellular_r4_api.c:451: if( ( tempValue >= MIN_TCP_SESSION_ID ) && ( tempValue <= MAX_TCP_SESSION_ID ) )
./cellular_r4_urc_handler.c:395: if( ( tempValue >= MIN_TCP_SESSION_ID ) && ( tempValue <= MAX_TCP_SESSION_ID ) )
./cellular_r4_urc_handler.c:490: if( ( tempValue >= MIN_TCP_SESSION_ID ) && ( tempValue <= MAX_TCP_SESSION_ID ) )
./cellular_r4_urc_handler.c:554: if( ( tempValue >= MIN_TCP_SESSION_ID ) && ( tempValue <= MAX_TCP_SESSION_ID ) )
The usage here is to compare int32_t ( result of Cellular_ATStrtoi ).
Please suggest the modification.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Original warning message.
[ 19%] Built target cmock
/mnt/c/msys64/home/chinglee/cellular/public/chinglee/FreeRTOS-Cellular-Interface/modules/sara_r4/cellular_r4.c: In function ‘_Cellular_GetSocketId’:
/mnt/c/msys64/home/chinglee/cellular/public/chinglee/FreeRTOS-Cellular-Interface/modules/sara_r4/cellular_r4.c:496:21: warning: comparison is always true due to limited range of data type [-Wtype-limits]
496 | ( sessionId >= MIN_TCP_SESSION_ID ) && ( sessionId <= MAX_TCP_SESSION_ID ) ) {
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Update the type comparison MAX_TCP_SESSION_ID in next commit.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good to me.