CID1129998/1129999: Unchecked return value from library
The function returns a value that indicates an error condition. If this is not checked, the error condition may not be handled correctly. In serialport_write_nonblock: Value returned from a library function is not checked for errors before being used. This value may indicate an error condition. In serialport_config: Value returned from a library function is not checked for errors before being used. This value may indicate an error condition.
Signed-off-by: Stefan Reinauer stefan.reinauer@coreboot.org
Index: serial.c =================================================================== --- serial.c (revision 1763) +++ serial.c (working copy) @@ -181,7 +181,10 @@ msg_pdbg("Baud rate is %ld.\n", dcb.BaudRate); #else struct termios wanted, observed; - fcntl(fd, F_SETFL, 0); + if (fcntl(fd, F_SETFL, 0) != 0) { + msg_perr_strerror("Could not set serial port mode: "); + return 1; + } if (tcgetattr(fd, &observed) != 0) { msg_perr_strerror("Could not fetch original serial port configuration: "); return 1; @@ -419,14 +422,21 @@ return -1; } if(!SetCommTimeouts(sp_fd, &newTimeout)) { + msg_perr_strerror("Could not set serial port timeout settings: "); + return -1; + } #else ssize_t rv; const int flags = fcntl(sp_fd, F_GETFL); + if (flags == -1) { + msg_perr_strerror("Could not get serial port mode: "); + return -1; + } if (fcntl(sp_fd, F_SETFL, flags | O_NONBLOCK) != 0) { -#endif - msg_perr_strerror("Could not set serial port timeout settings %s"); + msg_perr_strerror("Could not set serial port mode to non-blocking: "); return -1; } +#endif
int i; int rd_bytes = 0; @@ -458,12 +468,15 @@ /* restore original blocking behavior */ #ifdef _WIN32 if (!SetCommTimeouts(sp_fd, &oldTimeout)) { + msg_perr_strerror("Could not set serial port timeout settings: "); + ret = -1; + } #else if (fcntl(sp_fd, F_SETFL, flags) != 0) { -#endif - msg_perr_strerror("Could not restore serial port timeout settings: %s\n"); + msg_perr_strerror("Could not set serial port mode to non-blocking: "); ret = -1; } +#endif return ret; }
@@ -495,7 +508,14 @@ #else ssize_t rv; const int flags = fcntl(sp_fd, F_GETFL); - fcntl(sp_fd, F_SETFL, flags | O_NONBLOCK); + if (flags == -1) { + msg_perr_strerror("Could not get serial port mode: "); + return -1; + } + if (fcntl(sp_fd, F_SETFL, flags | O_NONBLOCK) != 0) { + msg_perr_strerror("Could not set serial port mode to non-blocking: "); + return -1; + } #endif
int i; @@ -531,10 +551,13 @@ #ifdef _WIN32 if (!SetCommTimeouts(sp_fd, &oldTimeout)) { msg_perr_strerror("Could not restore serial port timeout settings: "); + return -1; + } #else if (fcntl(sp_fd, F_SETFL, flags) != 0) { -#endif + msg_perr_strerror("Could not reset serial port mode: "); return -1; } +#endif return ret; }
On Tue, 19 Nov 2013 20:35:57 +0100 Stefan Reinauer stefan.reinauer@coreboot.org wrote:
Index: serial.c
--- serial.c (revision 1763) +++ serial.c (working copy) @@ -181,7 +181,10 @@ msg_pdbg("Baud rate is %ld.\n", dcb.BaudRate); #else struct termios wanted, observed;
- fcntl(fd, F_SETFL, 0);
- if (fcntl(fd, F_SETFL, 0) != 0) {
msg_perr_strerror("Could not set serial port mode: ");return 1;- } if (tcgetattr(fd, &observed) != 0) { msg_perr_strerror("Could not fetch original serial port configuration: "); return 1;
Why do we clear all file descriptor flags anyway? The code is from Urja, hence the CC.
On Sat, 26 Apr 2014 13:28:51 +0200 Stefan Tauner stefan.tauner@alumni.tuwien.ac.at wrote:
On Tue, 19 Nov 2013 20:35:57 +0100 Stefan Reinauer stefan.reinauer@coreboot.org wrote:
Index: serial.c
--- serial.c (revision 1763) +++ serial.c (working copy) @@ -181,7 +181,10 @@ msg_pdbg("Baud rate is %ld.\n", dcb.BaudRate); #else struct termios wanted, observed;
- fcntl(fd, F_SETFL, 0);
- if (fcntl(fd, F_SETFL, 0) != 0) {
msg_perr_strerror("Could not set serial port mode: ");return 1;- } if (tcgetattr(fd, &observed) != 0) { msg_perr_strerror("Could not fetch original serial port configuration: "); return 1;
Why do we clear all file descriptor flags anyway? The code is from Urja, hence the CC.
Whatever the reason is, I have committed the patch (with minor refinements) in r1803. I think that was the last bug fix originating in coverity scans. Thanks again Stefan!
-
Stefan Reinauer -
Stefan Tauner