From 7679f8f88622cd14d442e69d133bd108783cdd80 Mon Sep 17 00:00:00 2001 From: David Steele Date: Sat, 4 Apr 2020 13:59:50 -0400 Subject: [PATCH] Fix issue with THROW*_SYS_ERROR_CODE*() using wrong errno. When DEBUG_COVERAGE was defined errno was being used instead of the value being passed. This apparently worked by happenstance in the single existing usage but it won't work in general. --- src/common/error.c | 10 +---- src/common/error.h | 98 ++++++++++++++++++++++++++++------------------ 2 files changed, 62 insertions(+), 46 deletions(-) diff --git a/src/common/error.c b/src/common/error.c index 944a566e9..0baec2406 100644 --- a/src/common/error.c +++ b/src/common/error.c @@ -388,15 +388,12 @@ void errorInternalThrowSys( #ifdef DEBUG_COVERAGE bool error, -#else - int errNo, #endif - const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message) + int errNo, const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message) { #ifdef DEBUG_COVERAGE if (error) { - int errNo = errno; #endif // Format message with system message appended @@ -419,15 +416,12 @@ void errorInternalThrowSysFmt( #ifdef DEBUG_COVERAGE bool error, -#else - int errNo, #endif - const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *format, ...) + int errNo, const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *format, ...) { #ifdef DEBUG_COVERAGE if (error) { - int errNo = errno; #endif // Format message diff --git a/src/common/error.h b/src/common/error.h index a4a45666a..4e54e57b7 100644 --- a/src/common/error.h +++ b/src/common/error.h @@ -180,25 +180,52 @@ Throw an error when a system call fails // THROW*_ON*() calls that contain conditionals. #ifdef DEBUG_COVERAGE #define THROW_SYS_ERROR(errorType, message) \ - errorInternalThrowSys(true, &errorType, __FILE__, __func__, __LINE__, message) + errorInternalThrowSys(true, errno, &errorType, __FILE__, __func__, __LINE__, message) #define THROW_SYS_ERROR_FMT(errorType, ...) \ - errorInternalThrowSysFmt(true, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + errorInternalThrowSysFmt(true, errno, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) #define THROWP_SYS_ERROR(errorType, message) \ - errorInternalThrowSys(true, errorType, __FILE__, __func__, __LINE__, message) + errorInternalThrowSys(true, errno, errorType, __FILE__, __func__, __LINE__, message) #define THROWP_SYS_ERROR_FMT(errorType, ...) \ - errorInternalThrowSysFmt(true, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + errorInternalThrowSysFmt(true, errno, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) - #define THROW_ON_SYS_ERROR(error, errorType, message) \ - errorInternalThrowSys(error, &errorType, __FILE__, __func__, __LINE__, message) + // The expression can't be passed directly to errorInternalThrowSys*() because we need to be sure it is evaluated before passing + // errno. Depending on optimization that might not happen. + #define THROW_ON_SYS_ERROR(expression, errorType, message) \ + do \ + { \ + bool error = expression; \ + errorInternalThrowSys(error, errno, &errorType, __FILE__, __func__, __LINE__, message); \ + } while(0) - #define THROW_ON_SYS_ERROR_FMT(error, errorType, ...) \ - errorInternalThrowSysFmt(error, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + #define THROW_ON_SYS_ERROR_FMT(expression, errorType, ...) \ + do \ + { \ + bool error = expression; \ + errorInternalThrowSysFmt(error, errno, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__); \ + } while(0) - #define THROWP_ON_SYS_ERROR(error, errorType, message) \ - errorInternalThrowSys(error, errorType, __FILE__, __func__, __LINE__, message) + #define THROWP_ON_SYS_ERROR(expression, errorType, message) \ + do \ + { \ + bool error = expression; \ + errorInternalThrowSys(error, errno, errorType, __FILE__, __func__, __LINE__, message); \ + } while(0) - #define THROWP_ON_SYS_ERROR_FMT(error, errorType, ...) \ - errorInternalThrowSysFmt(error, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + #define THROWP_ON_SYS_ERROR_FMT(expression, errorType, ...) \ + do \ + { \ + bool error = expression; \ + errorInternalThrowSysFmt(error, errno, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__); \ + } while(0) + + #define THROW_SYS_ERROR_CODE(errNo, errorType, message) \ + errorInternalThrowSys(true, errNo, &errorType, __FILE__, __func__, __LINE__, message) + #define THROW_SYS_ERROR_CODE_FMT(errNo, errorType, ...) \ + errorInternalThrowSysFmt(true, errNo, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + #define THROWP_SYS_ERROR_CODE(errNo, errorType, message) \ + errorInternalThrowSys(true, errNo, errorType, __FILE__, __func__, __LINE__, message) + #define THROWP_SYS_ERROR_CODE_FMT(errNo, errorType, ...) \ + errorInternalThrowSysFmt(true, errNo, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) // Else define the normal macros which check for an error first #else @@ -211,43 +238,43 @@ Throw an error when a system call fails #define THROWP_SYS_ERROR_FMT(errorType, ...) \ errorInternalThrowSysFmt(errno, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) - #define THROW_ON_SYS_ERROR(error, errorType, message) \ + #define THROW_ON_SYS_ERROR(expression, errorType, message) \ do \ { \ - if (error) \ + if (expression) \ errorInternalThrowSys(errno, &errorType, __FILE__, __func__, __LINE__, message); \ } while(0) - #define THROW_ON_SYS_ERROR_FMT(error, errorType, ...) \ + #define THROW_ON_SYS_ERROR_FMT(expression, errorType, ...) \ do \ { \ - if (error) \ + if (expression) \ errorInternalThrowSysFmt(errno, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__); \ } while(0) - #define THROWP_ON_SYS_ERROR(error, errorType, message) \ + #define THROWP_ON_SYS_ERROR(expression, errorType, message) \ do \ { \ - if (error) \ + if (expression) \ errorInternalThrowSys(errno, errorType, __FILE__, __func__, __LINE__, message); \ } while(0) - #define THROWP_ON_SYS_ERROR_FMT(error, errorType, ...) \ + #define THROWP_ON_SYS_ERROR_FMT(expression, errorType, ...) \ do \ { \ - if (error) \ + if (expression) \ errorInternalThrowSysFmt(errno, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__); \ } while(0) -#endif -#define THROW_SYS_ERROR_CODE(errNo, errorType, message) \ - errorInternalThrowSys(errNo, &errorType, __FILE__, __func__, __LINE__, message) -#define THROW_SYS_ERROR_CODE_FMT(errNo, errorType, ...) \ - errorInternalThrowSysFmt(errNo, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) -#define THROWP_SYS_ERROR_CODE(errNo, errorType, message) \ - errorInternalThrowSys(errNo, errorType, __FILE__, __func__, __LINE__, message) -#define THROWP_SYS_ERROR_CODE_FMT(errNo, errorType, ...) \ - errorInternalThrowSysFmt(errNo, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + #define THROW_SYS_ERROR_CODE(errNo, errorType, message) \ + errorInternalThrowSys(errNo, &errorType, __FILE__, __func__, __LINE__, message) + #define THROW_SYS_ERROR_CODE_FMT(errNo, errorType, ...) \ + errorInternalThrowSysFmt(errNo, &errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) + #define THROWP_SYS_ERROR_CODE(errNo, errorType, message) \ + errorInternalThrowSys(errNo, errorType, __FILE__, __func__, __LINE__, message) + #define THROWP_SYS_ERROR_CODE_FMT(errNo, errorType, ...) \ + errorInternalThrowSysFmt(errNo, errorType, __FILE__, __func__, __LINE__, __VA_ARGS__) +#endif /*********************************************************************************************************************************** Rethrow the current error @@ -293,10 +320,8 @@ void errorInternalThrowFmt( void errorInternalThrowSys( #ifdef DEBUG_COVERAGE bool error, -#else - int errNo, #endif - const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message) + int errNo, const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *message) #ifdef DEBUG_COVERAGE ; #else @@ -306,15 +331,12 @@ void errorInternalThrowSys( void errorInternalThrowSysFmt( #ifdef DEBUG_COVERAGE bool error, -#else - int errNo, #endif - const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *format, ...) - __attribute__((format(printf, 6, 7))) + int errNo, const ErrorType *errorType, const char *fileName, const char *functionName, int fileLine, const char *format, ...) #ifdef DEBUG_COVERAGE - ; + __attribute__((format(printf, 7, 8))); #else - __attribute__((__noreturn__)); + __attribute__((format(printf, 6, 7))) __attribute__((__noreturn__)); #endif /***********************************************************************************************************************************