From ad551b363eb5aa1f464488cd189f655b5587625d Mon Sep 17 00:00:00 2001 From: Martin Lund Date: Mon, 9 May 2016 17:28:43 +0200 Subject: [PATCH] Cleanup of error handling Introduced consistent way of handling errors and printing error messages. Also upgraded some warnings to errors. --- src/error.c | 5 +- src/include/tio/error.h | 5 +- src/main.c | 9 +-- src/options.c | 26 ++++---- src/time.c | 3 +- src/tty.c | 139 +++++++++++++++++++++++----------------- 6 files changed, 106 insertions(+), 81 deletions(-) diff --git a/src/error.c b/src/error.c index 31cf870..36e187b 100644 --- a/src/error.c +++ b/src/error.c @@ -25,11 +25,12 @@ #include #include "tio/options.h" #include "tio/print.h" +#include "tio/error.h" -char *error = ""; +char error[1000]; void error_exit(void) { if ((error[0] != 0) && (option.no_autoconnect)) - printf("Error: %s\n", error); + printf("\rError: %s\r\n", error); } diff --git a/src/include/tio/error.h b/src/include/tio/error.h index e7609b1..3375473 100644 --- a/src/include/tio/error.h +++ b/src/include/tio/error.h @@ -25,7 +25,10 @@ #define TIO_SUCCESS 0 #define TIO_ERROR 1 -extern char *error; +extern char error[1000]; + +#define error_printf(format, args...) \ + snprintf (error, 1000, format, ## args); void error_exit(void); diff --git a/src/main.c b/src/main.c index cdb2d0b..4a20559 100644 --- a/src/main.c +++ b/src/main.c @@ -32,21 +32,18 @@ int main(int argc, char *argv[]) { int status; + /* Install error exit handler */ + atexit(&error_exit); + /* Parse options */ parse_options(argc, argv); /* Configure output terminal */ configure_stdout(); - /* Install error exit handler */ - atexit(&error_exit); - /* Install log exit handler */ atexit(&log_exit); - /* Restore output terminal on exit */ - atexit(&restore_stdout); - /* Create log file */ if (option.log) log_open(option.log_filename); diff --git a/src/options.c b/src/options.c index 008d354..b58dac8 100644 --- a/src/options.c +++ b/src/options.c @@ -30,7 +30,7 @@ #include #include "config.h" #include "tio/options.h" -#include "tio/print.h" +#include "tio/error.h" struct option_t option = { @@ -71,7 +71,7 @@ void parse_options(int argc, char *argv[]) if (argc == 1) { print_options_help(argv); - exit(0); + exit(EXIT_SUCCESS); } /* Set default termios settings: @@ -213,7 +213,7 @@ void parse_options(int argc, char *argv[]) baudrate = B4000000; break; default: - printf("Error: Invalid baud rate.\n"); + error_printf("Invalid baud rate"); exit(EXIT_FAILURE); } @@ -240,7 +240,7 @@ void parse_options(int argc, char *argv[]) option.tio.c_cflag |= CS8; break; default: - printf("Error: Invalid data bits.\n"); + error_printf("Invalid data bits"); exit(EXIT_FAILURE); } break; @@ -263,7 +263,7 @@ void parse_options(int argc, char *argv[]) } else { - printf("Error: Invalid flow control.\n"); + error_printf("Invalid flow control"); exit(EXIT_FAILURE); } break; @@ -279,7 +279,7 @@ void parse_options(int argc, char *argv[]) option.tio.c_cflag |= CSTOPB; break; default: - printf("Error: Invalid stop bits.\n"); + error_printf("Invalid stop bits"); exit(EXIT_FAILURE); } break; @@ -299,7 +299,7 @@ void parse_options(int argc, char *argv[]) option.tio.c_cflag &= ~PARENB; else { - printf("Error: Invalid parity.\n"); + error_printf("Invalid parity"); exit(EXIT_FAILURE); } break; @@ -324,21 +324,21 @@ void parse_options(int argc, char *argv[]) printf("License GPLv2: GNU GPL version 2 or later .\n"); printf("This is free software: you are free to change and redistribute it.\n"); printf("There is NO WARRANTY, to the extent permitted by law.\n"); - exit(0); + exit(EXIT_SUCCESS); break; case 'h': print_options_help(argv); - exit(0); + exit(EXIT_SUCCESS); break; case '?': /* getopt_long already printed an error message */ - exit(1); + exit(EXIT_FAILURE); break; default: - exit(1); + exit(EXIT_FAILURE); } } @@ -348,14 +348,14 @@ void parse_options(int argc, char *argv[]) if (strlen(option.tty_device) == 0) { - printf("Error: Missing device name.\n"); + error_printf("Missing device name"); exit(EXIT_FAILURE); } /* Print any remaining command line arguments (unknown options) */ if (optind < argc) { - printf("%s: unknown arguments: ", argv[0]); + printf("Error: unknown arguments: "); while (optind < argc) printf("%s ", argv[optind++]); printf("\n"); diff --git a/src/time.c b/src/time.c index 4ab5eea..7132248 100644 --- a/src/time.c +++ b/src/time.c @@ -22,6 +22,7 @@ #include #include #include +#include "tio/error.h" char * current_time(void) { @@ -33,7 +34,7 @@ char * current_time(void) tmp = localtime(&t); if (tmp == NULL) { - printf("Error: Retrieving local time failed\n"); + error_printf("Retrieving local time failed"); exit(EXIT_FAILURE); } diff --git a/src/tty.c b/src/tty.c index 65f08fa..7449919 100644 --- a/src/tty.c +++ b/src/tty.c @@ -46,7 +46,7 @@ static int fd; void wait_for_tty_device(void) { fd_set rdfs; - int ready, status; + int status; struct timeval tv; static char c_stdin[3]; static bool first = true; @@ -71,21 +71,29 @@ void wait_for_tty_device(void) FD_SET(STDIN_FILENO, &rdfs); /* Block until input becomes available or timeout */ - ready = select(STDIN_FILENO + 1, &rdfs, NULL, NULL, &tv); - if (ready) + status = select(STDIN_FILENO + 1, &rdfs, NULL, NULL, &tv); + if (status) { /* Input from stdin ready */ /* Read one character */ status = read(STDIN_FILENO, &c_stdin[0], 1); if (status < 0) - printf("Warning: Could not read from stdin\r\n"); + { + error_printf("Could not read from stdin"); + exit(EXIT_FAILURE); + } /* Exit upon ctrl-t + q sequence */ c_stdin[2] = c_stdin[1]; c_stdin[1] = c_stdin[0]; if ((c_stdin[1] == KEY_Q) && (c_stdin[2] == KEY_CTRL_T)) exit(EXIT_SUCCESS); + + } else if (status == -1) + { + error_printf("select() failed (%s)", strerror(errno)); + exit(EXIT_FAILURE); } /* Test for accessible device file */ @@ -99,7 +107,7 @@ void configure_stdout(void) /* Save current stdout settings */ if (tcgetattr(STDOUT_FILENO, &old_stdout) < 0) { - printf("Error: Saving current stdio settings failed\n"); + error_printf("Saving current stdio settings failed"); exit(EXIT_FAILURE); } @@ -119,11 +127,13 @@ void configure_stdout(void) /* Activate new stdout settings */ tcsetattr(STDOUT_FILENO, TCSANOW, &new_stdout); tcsetattr(STDOUT_FILENO, TCSAFLUSH, &new_stdout); + + /* Make sure we restore old stdout settings on exit */ + atexit(&restore_stdout); } void restore_stdout(void) { - tcflush(STDOUT_FILENO, TCIOFLUSH); tcsetattr(STDOUT_FILENO, TCSANOW, &old_stdout); tcsetattr(STDOUT_FILENO, TCSAFLUSH, &old_stdout); } @@ -132,9 +142,13 @@ void disconnect_tty(void) { if (tainted) putchar('\n'); - color_printf("[tio %s] Disconnected", current_time()); - close(fd); - connected = false; + + if (connected) + { + color_printf("[tio %s] Disconnected", current_time()); + close(fd); + connected = false; + } } void restore_tty(void) @@ -159,14 +173,14 @@ int connect_tty(void) fd = open(option.tty_device, O_RDWR | O_NOCTTY ); if (fd < 0) { - error = strerror(errno); + error_printf("Could not open tty device (%s)", strerror(errno)); goto error_open; } /* Make sure device is of tty type */ if (!isatty(fd)) { - error = "Not a tty device"; + error_printf("Not a tty device"); goto error_isatty; } @@ -214,58 +228,68 @@ int connect_tty(void) FD_SET(STDIN_FILENO, &rdfs); /* Block until input becomes available */ - select(maxfd, &rdfs, NULL, NULL, NULL); - if (FD_ISSET(fd, &rdfs)) + status = select(maxfd, &rdfs, NULL, NULL, NULL); + if (status) { - /* Input from tty device ready */ - if (read(fd, &c_tty, 1) > 0) + if (FD_ISSET(fd, &rdfs)) { - /* Print received tty character to stdout */ - putchar(c_tty); - fflush(stdout); + /* Input from tty device ready */ + if (read(fd, &c_tty, 1) > 0) + { + /* Print received tty character to stdout */ + putchar(c_tty); + fflush(stdout); + + /* Write to log */ + if (option.log) + log_write(c_tty); + + tainted = true; + } else + { + /* Error reading - device is likely unplugged */ + error_printf("Could not read from tty device"); + goto error_read; + } + } + if (FD_ISSET(STDIN_FILENO, &rdfs)) + { + /* Input from stdin ready */ + status = read(STDIN_FILENO, &c_stdin[0], 1); + if (status < 0) + { + error_printf("Could not read from stdin"); + goto error_read; + } + + /* Exit upon ctrl-t + q sequence */ + c_stdin[2] = c_stdin[1]; + c_stdin[1] = c_stdin[0]; + if ((c_stdin[1] == KEY_Q) && (c_stdin[2] == KEY_CTRL_T)) + exit(EXIT_SUCCESS); + + /* Ignore ctrl-t except when repeated */ + if ((c_stdin[0] != KEY_CTRL_T) || + ((c_stdin[1] == KEY_CTRL_T) && (c_stdin[2] == KEY_CTRL_T))) + { + /* Forward input to tty device */ + status = write(fd, &c_stdin[0], 1); + if (status < 0) + printf("Warning: Could not write to tty device\r\n"); + } /* Write to log */ if (option.log) - log_write(c_tty); + log_write(c_stdin[0]); - tainted = true; - } else - { - /* Error reading - device is likely unplugged */ - disconnect_tty(); - goto error_reading; + /* Insert output delay */ + if (option.output_delay) + usleep(option.output_delay * 1000); } - } - if (FD_ISSET(STDIN_FILENO, &rdfs)) + } else if (status == -1) { - /* Input from stdin ready */ - status = read(STDIN_FILENO, &c_stdin[0], 1); - if (status < 0) - printf("Warning: Could not read from stdin\r\n"); - - /* Exit upon ctrl-t + q sequence */ - c_stdin[2] = c_stdin[1]; - c_stdin[1] = c_stdin[0]; - if ((c_stdin[1] == KEY_Q) && (c_stdin[2] == KEY_CTRL_T)) - exit(EXIT_SUCCESS); - - /* Ignore ctrl-t except when repeated */ - if ((c_stdin[0] != KEY_CTRL_T) || - ((c_stdin[1] == KEY_CTRL_T) && (c_stdin[2] == KEY_CTRL_T))) - { - /* Forward input to tty device */ - status = write(fd, &c_stdin[0], 1); - if (status < 0) - printf("Warning: Could not write to tty device\r\n"); - } - - /* Write to log */ - if (option.log) - log_write(c_stdin[0]); - - /* Insert output delay */ - if (option.output_delay) - usleep(option.output_delay * 1000); + error_printf("Error: select() failed (%s)", strerror(errno)); + exit(EXIT_FAILURE); } } @@ -273,9 +297,8 @@ int connect_tty(void) error_tcgetattr: error_isatty: - close(fd); - connected = false; -error_reading: +error_read: + disconnect_tty(); error_open: return TIO_ERROR; }