Undefined variable error in C macros (for customized prints)

Question

I'm trying to make a custom printf that prints the file / line no , along with the error message , depending on the current print level set. I've defined a macro for the same. Given below is the code for the preprocessor:

#define DIE (s) \
    printf(s); \
    exit(0); \

#define my_print(level,s) \
if(level <= gPrintLevel) \
{ \
    char *buffer = (char *)malloc(strlen(s)-1); \
    if (NULL != buffer) \
    { \
       sprintf(buffer,s); \
       printf("[%s][%d]:%s\n",__FUNCTION__,__LINE__,buffer); \
       if (level == fatal) \
       {\
          DIE(s);\
       }\
    } \
} \

I'm calling the above pre-processor like this from inside a function:

myPrint(2,"Unexpected error encountered\n");

But, I'm getting the below compile errors when I try to compile:

41: error: ‘s’ was not declared in this scope

Please help, what am I doing wrong ? Also, its appreciated if someone can tell me if there's a more elegant way of having customized print statements as above. Thanks in advance.

And [don't cast the return value of `malloc()`](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858)! — , Oct 20 '13 at 10:51
This doesn't appear to be your actual code (`myPrint != my_print`). Please paste your actual code. Also, which is line 41? — Oliver Charlesworth, Oct 20 '13 at 10:52
@H2CO3 How would `__FUNCTION__` and `__LINE__` be meaningful with a static inline method? — Daniel Frey, Oct 20 '13 at 10:53
As an aside, you don't seem to be freeing your memory anywhere. Also, why do you need this extra buffer at all? — Oliver Charlesworth, Oct 20 '13 at 10:53
@DanielFrey Function arguments to the rescue! `die(__FUNCTION__, __LINE__);`... — , Oct 20 '13 at 10:55
@H2CO3 The question is labeled C and C++, which presumably means that he needs a solution which works in both languages. Which means casting the return value of `malloc`. — James Kanze, Oct 20 '13 at 11:00
@JamesKanze Tagging a question with both C and C++ at the same time is wrong. No C code should be compiled as C++ and vice versa. — , Oct 20 '13 at 11:01
I don't know about the variable, but some notes -- first, the same items @OliCharlesworth mentioned. Second, your copy of the (unnecessary?) buffer lacks NULL termination... use `buffer = strdup(s);` instead (or better yet, don't even make a copy). — mah, Oct 20 '13 at 11:01
thanks for the super quick responses, that myPrint/my_print was a typo error, I couldn't copy/paste the actual code, as there's some issue from copy/pasting from my virtualbox linux terminal to the windows browser . Anyway, I'm using malloc to avoid having a static char array to copy the contents using sprintf. — vinit, Oct 20 '13 at 11:05
@H2CO3 You could implement both functions as static inline methods, but you still want a macro as you don't want to pass in `__FUNCTION__` and `__LINE__` in each call directly. Or just use KerrekSB's answer :) — Daniel Frey, Oct 20 '13 at 11:06
@H2CO3 A lot of *code* from header files is compiled as C and C++ headers every day. Of course most of the content is just function prototypes but macros and static inline functions can be there as well. — Pavel Šimerda, Oct 20 '13 at 11:07
@DanielFrey Actually, in real life, I'd go with a simple macro that expands `__FUNCTION__` and `__LINE__` in a **readable** way, and then passes those to a private handler function that does the real work (IIRC that's how e. g. `assert()` is implemented in most libc implementations). — , Oct 20 '13 at 11:09
@PavelŠimerda I'm not saying that "no C code **is** compiled as C++". I'm saying that "no C code **should be** compiled as C++". As to the header files: there's a common subset of C and C++, in which you write code, then it works in both languages (and it may even work correctly). `malloc()`, when used in the idiomatic (==right) way, is not included in this common subset. — , Oct 20 '13 at 11:11
@H2CO3 That is what I'm doing as well: Let the macro do the bare minimum that only a macro *can* do, move the rest to a proper implementation function. — Daniel Frey, Oct 20 '13 at 11:13
@H2CO3 I understood you, no clarifications needed. The malloc result typecasting is indeed redundant in C, but not incorrect, especially if your C code is meant to be compatible with C++ (i.e. if it is written in a subset of C and C++ using macros to distinguish the two when necessary). Technology and religion shouldn't be mixed together. — Pavel Šimerda, Oct 20 '13 at 11:32
@H2CO3 That's nonsense. There are plenty of cases where code must be compiled both as C++ and as C. This code is clearly meant to be in a header, which could easily be common to both C and C++. — James Kanze, Oct 20 '13 at 12:41

Kerrek SB · Answer 1 · 2013-10-20T16:59:54.633

4

Personally, I would simply assume or mandate that the user provide a literal format string. In that case, you can concatenate strings:

#define MYPRINT(fmt, ...) \
  printf("Function: %s. Line: %d. " fmt "\n", \
         __FUNCTION__, __LINE__, ## __VA_ARGS__);

Usage:

MYPRINT("The flargle %d has unexpected grobule %f", f->q, f->r);

This approach also lets you take advantage of the compiler's ability to analyze the format string statically and warn you about mismatching arguments.

(The code uses a GCC extension involving ## to elide the final comma in case the argument list is empty.)

edited Oct 20 '13 at 16:59

answered Oct 20 '13 at 10:53

Kerrek SB

464,522
92
875
1,084

This sounds good, but I also want to have a level id to know if I should print this or not. Can I use the abovementioned MYPRINT like this : MYPRINT(level,fmt,....) { if (level > currentPrintLevel) printf(.....) } ? – vinit Oct 20 '13 at 11:08
@wildplasser: I believe I am stating that. – Kerrek SB Oct 20 '13 at 13:39
@PavelŠimerda: Then you should make an `DIE_LOCALLY` macro that passes the magic values through to your `gettext` or whatever string lookup, I suppose. – Kerrek SB Oct 20 '13 at 13:40
@wildplasser, i didn't get you, can u elaborate, what will not work ? tia ! – vinit Oct 20 '13 at 14:03
It will not work if `fmt` is **not** a string literal (as @KerrekSB already stated in his answer) The caller _could_ construct the format string in a character buffer and pass a pointer to that buffer to the debug macro. (The good news: the compiler would not accept such a program. The bad news: debugging macros is hard. Debugging debugging macros is even harder) – wildplasser Oct 20 '13 at 14:12
@KerrekSB: Sounds like undue complexity. – Pavel Šimerda Oct 20 '13 at 15:47
@PavelŠimerda: Sure - but so does localizing fatal debug messages :-) – Kerrek SB Oct 20 '13 at 16:58

score 0 · Answer 2 · answered Oct 20 '13 at 13:01

0

OK thanks for all the help guys, the variadic macros solution works fine. This is the new defn of the macro now:

#define DIE(fmt) \
do { \
printf(fmt); \
exit(0); \
} while(0); \

#define my_print(x,fmt,...) \
if (x < gPrintLevel) \
{ \
    printf("[%s][%u]:" fmt "\n",__FUNCTION__,__LINE__,##__VA_ARGS__); \
    if (fatal == x) \
    {\
       DIE(fmt) \
    }\
} \

answered Oct 20 '13 at 13:01

vinit

501
5
15

It looks like `DIE` shouldn't need to print the message again. `printf(fmt)` definitely looks like trouble (maybe `puts` would be better?). And you should wrap the `my_print` macro in a `do { ... } while (false)`, too. – Kerrek SB Oct 20 '13 at 17:02
@KerrekSB, you are right, removed the print from DIE, but why do we need do {....}while(false) ? – vinit Nov 04 '13 at 02:44

Undefined variable error in C macros (for customized prints)

2 Answers2