[PATCH] CID1129998/1129999: Unchecked return value from library

[Stefan Reinauer]

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 <[e-mail]>

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;
 }

[PATCH] CID1129998/1129999: Unchecked return value from library

[Stefan Tauner]

On Tue, 19 Nov 2013 20:35:57 +0100
Stefan Reinauer <[e-mail]> 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.
-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner

[PATCH] CID1129998/1129999: Unchecked return value from library

[Stefan Tauner]

On Sat, 26 Apr 2014 13:28:51 +0200
Stefan Tauner <[e-mail]> wrote:
> On Tue, 19 Nov 2013 20:35:57 +0100
> Stefan Reinauer <[e-mail]> 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!

-- 
Kind regards/Mit freundlichen Grüßen, Stefan Tauner