Update ArduinoOTA.cpp to solve #4086 #4090
Conversation
libraries/ArduinoOTA/ArduinoOTA.cpp
Outdated
| } | ||
| return 0; | ||
| } | ||
|
|
||
| String ArduinoOTAClass::readStringUntil(char end){ | ||
| String res = ""; | ||
| int value; | ||
| char value; |
There was a problem hiding this comment.
What is the motivation for this change?
There was a problem hiding this comment.
value should be declared int, because read() returns an int that can be -1 on error
There was a problem hiding this comment.
The motivation is back to original before yoursunny PR (it was char) and homogenize data types ( '/0' and end are char)
libraries/ArduinoOTA/ArduinoOTA.cpp
Outdated
| @@ -146,26 +146,26 @@ int ArduinoOTAClass::parseInt(){ | |||
| uint8_t index; | |||
| char value; | |||
| while(_udp_ota->peek() == ' ') _udp_ota->read(); | |||
| for(index = 0; index < sizeof(data); ++index){ | |||
| for(index = 0; index < sizeof(data); index++){ | |||
| value = _udp_ota->peek(); | |||
| if(value < '0' || value > '9'){ | |||
| data[index++] = '\0'; | |||
There was a problem hiding this comment.
For me, it would be even clearer to not increment the index here, something like this:
data[index+1] = '\0';
There was a problem hiding this comment.
This should be data[index] = '\0';
libraries/ArduinoOTA/ArduinoOTA.cpp
Outdated
| @@ -146,26 +146,26 @@ int ArduinoOTAClass::parseInt(){ | |||
| uint8_t index; | |||
| char value; | |||
| while(_udp_ota->peek() == ' ') _udp_ota->read(); | |||
| for(index = 0; index < sizeof(data); ++index){ | |||
| for(index = 0; index < sizeof(data); index++){ | |||
There was a problem hiding this comment.
This change is undesired (++index is better style in C++ in a for loop)
libraries/ArduinoOTA/ArduinoOTA.cpp
Outdated
| @@ -146,26 +146,26 @@ int ArduinoOTAClass::parseInt(){ | |||
| uint8_t index; | |||
| char value; | |||
There was a problem hiding this comment.
value should be declared int, because peek() returns an int that can be -1 on error
libraries/ArduinoOTA/ArduinoOTA.cpp
Outdated
| @@ -146,26 +146,26 @@ int ArduinoOTAClass::parseInt(){ | |||
| uint8_t index; | |||
| char value; | |||
| while(_udp_ota->peek() == ' ') _udp_ota->read(); | |||
| for(index = 0; index < sizeof(data); ++index){ | |||
| for(index = 0; index < sizeof(data); index++){ | |||
| value = _udp_ota->peek(); | |||
| if(value < '0' || value > '9'){ | |||
There was a problem hiding this comment.
This if statement needs a check against peek error return: if(value < 0 || everythingelse)
libraries/ArduinoOTA/ArduinoOTA.cpp
Outdated
| } | ||
| return 0; | ||
| } | ||
|
|
||
| String ArduinoOTAClass::readStringUntil(char end){ | ||
| String res = ""; | ||
| int value; | ||
| char value; |
There was a problem hiding this comment.
value should be declared int, because read() returns an int that can be -1 on error
libraries/ArduinoOTA/ArduinoOTA.cpp
Outdated
| while(true){ | ||
| value = _udp_ota->read(); | ||
| if(value < 0 || value == '\0' || value == end){ | ||
| if(value < '0' || value == '\0' || value == end){ |
There was a problem hiding this comment.
This change needs to be undon. The check value < 0 was meant to cover an error return from peek() or read(). Undo.
libraries/ArduinoOTA/ArduinoOTA.cpp
Outdated
| return res; | ||
| } | ||
| res += static_cast<char>(value); | ||
| res += value; |
There was a problem hiding this comment.
This change needs to be undone, given that value needs to be an int.
libraries/ArduinoOTA/ArduinoOTA.cpp
Outdated
| @@ -146,26 +146,26 @@ int ArduinoOTAClass::parseInt(){ | |||
| uint8_t index; | |||
| char value; | |||
| while(_udp_ota->peek() == ' ') _udp_ota->read(); | |||
| for(index = 0; index < sizeof(data); ++index){ | |||
| for(index = 0; index < sizeof(data); index++){ | |||
| value = _udp_ota->peek(); | |||
| if(value < '0' || value > '9'){ | |||
| data[index++] = '\0'; | |||
There was a problem hiding this comment.
This should be data[index] = '\0';
* Update ArduinoOTA.cpp * Update ArduinoOTA.cpp
Update ArduinoOTA.cpp to solve #4086