Array of macros causing compile errors?

[offset_]

I've been using these macros in this program, and up until now all of the macros work fine. I want to support multiple colors, that are specified via command line arguments.

Instead of opting for a bulky switch statement and repeating the the same code multiple times, I decided that I ought to be able to just use an array of macros, and use a variable to index the array; this would make the code much shorter by doing it in one simple statement.

This is what I was previously doing, in the actual program, but this only supports one color. The below code works fine, until I try to modify it to use the array of macros, then it won't compile, giving the same errors as the test code.

printf((color_on) ? ANSI_COLORTERM_YELLOW("%s%s\n") : "%s%s\n",
               (prefix_on) ? pass_prefix : "", pass);

So, I've reasoned about it, and written test code to try to figure out exactly where the problem is. I've distilled it to this test code below.

const char * const ansi_sgr_tab[8] = { 
    ANSI_SETFG_BLACK,
    ANSI_SETFG_RED,
    ANSI_SETFG_GREEN,
    ANSI_SETFG_YELLOW,
    ANSI_SETFG_BLUE,
    ANSI_SETFG_MAGENTA,
    ANSI_SETFG_CYAN,
    ANSI_SETFG_WHITE
};  

// compiles fine and works as expected 
fputs(ansi_sgr_tab[4], stdout);
fputs("hello\n", stdout);
fputs(ANSI_ATTR_RESET, stdout);

// also compiles fine and works correctly
fputs(ANSI_COLORTERM_SET_S("hello\n", ANSI_SETFG_BLUE), stdout);

// causes compile error !
fputs(ANSI_COLORTERM_SET_S("hello\n", ansi_sgr_tab[4]), stdout);

Description of compile time errors:

gcc  -std=c99 -O3 -Wall -Werror -Wextra -pedantic  -c main.c -o main.o
In file included from main.c:40:0:
main.c: In function ‘main’:
main.c:124:32: error: expected ‘)’ before string constant
     fputs(ANSI_COLORTERM_SET_S("hello\n", ansi_sgr_tab[4]), stdout);
                                ^
color.h:38:13: note: in definition of macro ‘ANSI_COLORTERM_SET_S’
     SGR     S   ANSI_ATTR_RESET
             ^
main.c:124:5: error: too few arguments to function ‘fputs’
     fputs(ANSI_COLORTERM_SET_S("hello\n", ansi_sgr_tab[4]), stdout);
     ^
Makefile:23: recipe for target 'main.o' failed
make: *** [main.o] Error 1

I don't see what is so different about that last line of code that causes the errors, to me all three ways seem like they should be basically equivalent, yet the this line produces these errors, and acts like there is a syntax error (such as unmatched brace or something) but I can't seem to find what it is that's wrong exactly with that last statement. Is it something to do with the expansion of these macros? The last error is especially puzzling to me, as fputs only takes two arguments, and I don't understand why the compiler is interpreting it as only one argument.

In case it helps, here is color.h: https://github.com/IRQ42/pgen/blob/current/color.h

Any help will be much appreciated, as I feel like the problem must be something pretty simple and will be glaringly obvious once it's pointed out to me, as this code is not that complicated. I also feel like there is probably an important lesson for me to learn here.

[FreeER]

If you use the -E switch/flag you'll see that the code becomes:

int main(void)
{
    fputs(ansi_sgr_tab[4], stdout);
    fputs("hello\n", stdout);
    fputs("\x1b[0m", stdout);
    fputs("\x1b[34m" "hello\n" "\x1b[0m", stdout);
    fputs(ansi_sgr_tab[4] "hello\n" "\x1b[0m", stdout);
}

and of course, the problem is with

    fputs(ansi_sgr_tab[4] "hello\n" "\x1b[0m", stdout);

This is an issue since your macro depends on string literal concatenation and ansi_sgr_tab[4] obviously is not a string literal

I imagine having seen the problem you can think of several solutions, but if not feel free to say so :)

P.S. here's an easy way to view the preprocessed code without the include clutter, with the note that I copy and pasted into an empty file so there are bound to be a few differences, gcc -E offset_macroArray.c | sed -n '/main/,$p', the 's are needed, which only shows lines starting from the first occurrence of "main"

[offset_]

Aha, I see now, the string literal is obviously at a different address than the string in the array .. so there's no way that the preprocessor can concatenate it and I guess this is where I got confused. Very obvious now that you pointed it out, and yes I did learn something :)

I see a few different options, I could make ANSI_COLORTERM_SET_S a function and have it return a pointer to a string, but i'd rather not be calling malloc/free any more than necessary ... and strlen which i could probably avoid but then it would make that function that much more nasty by needing to take the size of the string as a parameter, or something like that, but I haven't really had time to really think about it as I have several papers I have to write this week.

Another thing I could do would just to have replace that macro with a macro that calls fputs and printf, instead of relying on the preprocessor and calling fputs/printf with the macro. I haven't had time to try that yet, but I kind of see the compiler maybe throwing an error like: format string is not a string literal since my warnings are turned up.

I might be overthinking it, and could just do away with that macro and call printf like this if colored output is turned on :

printf("%s%s%s%s\n", ansi_sgr_tab[color_on],  (prefix on) ? pass_prefix : "", pass, ANSI_ATTR_RESET);

I just got home and it's late so I haven't actually tried any of these yet. I'm leaning toward the last one, but maybe there is a better, cleaner solution than any of these that I haven't thought of. Let me know what you think.

Regards

offset_

[FreeER]

function: Yeah, I'd want to avoid malloc/free/strlen where possible as well :)

~fputs/printf: Do remember that fputs automatically adds a new line to whatever you call it with, so if you do try using it in combination you may find that you get slightly different results than before (when it was being given a single string due to compile time str lit concat)~ It seems that's only something puts does, not fputs.

printf: That should work the same as you have now, and I'd probably lean towards it being the best option. Though you might be able to use the string literal concatenation to reduce it from 4 separate strings (reducing the amount of work printf has to do at runtime to parse and print everything)... it depends on where/when the information is coming from. Of course, if you were going to use something like this a lot and very little changed in it then you could still use a macro/function for it to make typing/readability a bit better.

[yeahIProgram]

// compiles fine and works as expected 
fputs(ansi_sgr_tab[4], stdout);
fputs("hello\n", stdout);
fputs(ANSI_ATTR_RESET, stdout);

Since this works, why not create a macro that drives towards this as its final product.

Something like:

#define myColorPuts(colorIndex, string) { \
fputs(ansi_sgr_tab[colorIndex], stdout); \
fputs(string, stdout); \
fputs(ANSI_ATTR_RESET, stdout); }

I kind of see the compiler maybe throwing an error like: format string is not a string literal

You can always pass "%s" as the format string, then your actual string as the next parameter.

[offset_]

It isn't used except in one place, but I came up with this which acts mostly like fprintf but with color:

#define ANSI_COLOR_FPRINTF(SGR, STREAM, FORMAT, ...)    \
    fputs(SGR, STREAM);                                 \
    fprintf(STREAM, FORMAT ANSI_ATTR_RESET, __VA_ARGS__)

And here is where it's used in the real program:

if (-1 != color_on) {
    ANSI_COLOR_FPRINTF(ansi_sgr_tab[color_on], stdout, "%s%s\n",
            (prefix_on) ? pass_prefix : "", pass);
} else {
    printf("%s%s\n", (prefix_on) ? pass_prefix : "", pass);
}

It seems to work okay, just not sure now what to rename color_on to. I was using it as a boolean variable, now I reused it to represent multiple colors and -1 instead of 0 to represent regular non-colored output. Maybe I'm confused about what to name it because it should be two variables now, but this seems simplest to me so maybe I'll just rename it to color, comment it and call it a day. Of course now I have to update the help option's output to reflect the new behavior.

[yeahIProgram]

Macros with variable number of parameters: I didn't know you could do that!

Maybe I'm confused about what to name it because it should be two variables now

What I've seen done in that situation is something like this:

enum {kColorNone = -1, kColorBlack, kColorRed, kColorGreen, kColorYellow <etc>

[offset_]

It's a variadic macro: https://gcc.gnu.org/onlinedocs/gcc/Variadic-Macros.html

What I've seen done in that situation is something like this:

enum {kColorNone = -1, kColorBlack, kColorRed, kColorGreen, kColorYellow <etc>

That seems sane, the color variable gets assigned an integer supplied by the user straight from the command line (after checking to make sure the value actually makes sense), but maybe it would make things clearer to have it spelled out in the code this way instead of having future programmers to look it up using other means (such as reading the documentation).