1

I am trying to print error messages using following method but keep getting error on __VA_ARGS__. Using ERROR_HANDLER() to check whether sensor_id is invalid in function:

sensor_value_t datamgr_get_avg(sensor_id_t sensor_id);


#define ERROR_HANDLER(condition,...)    do { \
                      if (condition) { \
                        printf("\nError: in %s - function %s at line %d: %s\n", __FILE__, __func__, __LINE__, __VA_ARGS__); \
                        exit(EXIT_FAILURE); \
                      } \
                    } while(0)

From comment

I am not sure how to use the ERROR_HANDLER. Do I have to test something with condition? Like:

ERROR_HANDLER(sensor_id != NULL, "invalid sensor_id");
Community
  • 1
  • 1
Gurpreet.S
  • 89
  • 1
  • 9
  • 1
    Your code compiles fine. You probably want `!condition` there. `ERROR_HANDLER(0, "some text");` works for me. Which error are you getting? I am getting: `Error: in /tmp/Test/Test/main.c - function main at line 25: some text`. – Stanislav Pankevich May 14 '18 at 20:13
  • I am not sure how to use the ERROR_HANDLER. Do I have to test something with condition? Like " ERROR_HANDLER(sensor_id != NULL", "invalid sensor_id")? – Gurpreet.S May 14 '18 at 20:18

2 Answers2

1

Not checked this but try:

sensor_value_t datamgr_get_avg(sensor_id_t sensor_id);


#define ERROR_HANDLER(condition,format,...)    do { \
                      if (condition) { \
                        printf("\nError: in %s - function %s at line %d:", __FILE__, __func__, __LINE__); \
                        printf(format, __VA_ARGS__); \
                        exit(EXIT_FAILURE); \
                      } \
                    } while(0)

Note the second new argument, which will take the format string. Then the second call to printf to use the format string after printing the front part of your error. You may want to add a further printf to add a new line.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278
lmcdowall
  • 423
  • 3
  • 11
1

Your existing macro assumes that the __VA_ARGS__ will consist of a single string (or, more pedantically accurately, the first element of __VA_ARGS__ must be a string (literal or char *) and any extra arguments will be evaluated but ignored). This isn't very sensible. There are two different ways of fixing it, depending on what you really want.

Single string argument

This option requires that a single string value is supplied, not a variable-length argument list. This is the solution suggested by DevSolar while I was typing this up.

#define ERROR_HANDLER(condition, msg) \
    do { \
        if (condition) { \
            fprintf(stderr, "\nError: in %s - function %s at line %d: %s\n", \
                    __FILE__, __func__, __LINE__, msg); \
            exit(EXIT_FAILURE); \
        } \
    } while(0)

There is ample precedent for this. One has only to look at the standard C _Static_assert declaration (aka static_assert) to see an example.

Note that I've refined the printing so that it goes to standard error; that's where error messages should be printed.

You might use this like:

ERROR_HANDLER(sensor_id == NULL, "Invalid sensor ID");

The condition is checked and if it evaluates to true, the message is printed.

Variable list of arguments including format string

The basic version of this is outlined by lachlan in their answer. However, apart from not printing to standard error (readily fixed), that solution doesn't handle the case of "I only want to print a fixed string". If you read C #define macro for debug printing, you will find a discussion of the intricacies of handling this optional arguments. In this context, however, it is sufficient to treat the format string as one of the __VA_ARGS__:

#define ERROR_HANDLER(condition, ...) \
    do { \
        if (condition) { \
            fprintf(stderr, "\nError: in %s - function %s at line %d:", \
                    __FILE__, __func__, __LINE__); \
            fprintf(stderr, __VA_ARGS__); \
            exit(EXIT_FAILURE); \
        } \
    } while(0)

This will accept:

ERROR_HANDLER(sensor_val < MIN_SENSOR_VAL || sensor_val > MAX_SENSOR_VAL,
              "out of bounds sensor reading %d - should be in range [%d..%d]\n",
              sensor_val, MIN_SENSOR_VAL, MAX_SENSOR_VAL);

as well as:

ERROR_HANDLER(sensor_id == NULL, "Invalid sensor ID\n");

You can decide whether the macro should ensure that there's a newline at the end of the message.

One residual problem now is that in a multi-threaded program, the operations in the two fprintf() statements will be separately controlled so that there is no interleaving of calls, but another thread might coopt standard error between the two calls. You can use POSIX flockfile() and funlockfile() to prevent any interleaving, but it soon becomes better to use a variable arguments function to do the job, leaving just the test and call in the macro.

You might also consider the logic of the assert macro, which also takes a 'condition' but only executes the error action if the condition is false — it asserts that the condition is true but takes abortive action if it is false. This will have the benefit of familiarity for experience programmers. (FWIW, I had to amend my answer during the 5-minute grace period to invert the range condition; I'd originally written the code using assert-mode thinking.) If you keep the current logic, maybe the name of the macro should become ERROR_IF(condition, ...) — it better explains what is intended.

Jonathan Leffler
  • 730,956
  • 141
  • 904
  • 1,278