Cleanly exit with valid error message on serial port error #37
Conversation
Log serial port messages to a category level that is enabled by default Update FAQ to explain how to fix serial port permission problems
| enum sp_return result; | ||
|
|
||
| result = sp_open(m8_port, SP_MODE_READ_WRITE); | ||
| if (check(result) != SP_OK) return NULL; |
There was a problem hiding this comment.
Since main.c is expecting a NULL return on error, I changed this to return NULL instead of exiting. This allows the program to cleanly close, flushing all buffers and closing all handles.
| @@ -84,21 +96,22 @@ static int check(enum sp_return result) { | |||
|
|
|||
| switch (result) { | |||
| case SP_ERR_ARG: | |||
| SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "Error: Invalid argument.\n"); | |||
| abort(); | |||
| SDL_LogError(SDL_LOG_CATEGORY_APPLICATION, "Error: Invalid argument.\n"); | |||
There was a problem hiding this comment.
I wasn't seeing any of these messages. According to SDL2/SDL_log.h, the default values log INFO and higher messages for APPLICATION category, but only CRITICAL or higher for (almost) everything else. So logging at SYSTEM level meant that these messages weren't being displayed by default, so I changed them.
| @@ -84,21 +96,22 @@ static int check(enum sp_return result) { | |||
|
|
|||
| switch (result) { | |||
| case SP_ERR_ARG: | |||
| SDL_LogError(SDL_LOG_CATEGORY_SYSTEM, "Error: Invalid argument.\n"); | |||
| abort(); | |||
There was a problem hiding this comment.
You rarely want to call abort() to exit a program--that's an "abnormal" process exit like it crashed and it even dumps a crash log. According to the man page, it doesn't even flush the open streams. It would have been better to call exit() here since that causes a normal program exit. That's the standard way to exit a program when you want to.
But even better (in my opinion) it to return an error so that the higher level callers are notified of the error and shutdown more gracefully, so that's what I've done here.
|
Looks good to me, thank you :) |
Cleanly exit with valid error message on serial port error
Log serial port messages to a category level that is enabled by default
Update FAQ to explain how to fix serial port permission problems