how to use c macro with three lines of statement in c

Question

I would like to understand how to efficiently use C macros in my code.Can someone guide me with respect to the below mentioned code?

#include<stdio.h>
#include<string.h>

void write_strongswan_conf(char buffer[1000],int buffer_size)
{
   FILE *fp;
   char delimiter[]="\n";
   fp = fopen( "strongswan.txt" , "a" );
   strcat(buffer,"\n");
   fwrite(buffer, 1 ,buffer_size+1, fp );
   printf("Data Written:%s",buffer);
   fclose(fp);
}

int main ()
{
    char wr_buffer[1000];
    int  wr_buffer_size =1;

    strcpy(wr_buffer,"STRONG SWAN CONFIGURATION FILE");
    wr_buffer_size = strlen(wr_buffer);
    write_strongswan_conf(wr_buffer,wr_buffer_size);

    strcpy(wr_buffer,"THIS IS THE SECOND LINE");
    wr_buffer_size = strlen(wr_buffer);
    write_strongswan_conf(wr_buffer,wr_buffer_size);

   return(0);
}

This is a small program to append new strings to the end of a file and i have around 25 strings to be added to the file.As of now, I have to repeatedly write the below mentioned three lines to get it done.If my understanding is correct we will be able to reduce the LOC by implementing a macro for these three lines.

Can someone guide me ?

    strcpy(wr_buffer,"STRONG SWAN CONFIGURATION FILE");
    wr_buffer_size = strlen(wr_buffer);
    write_strongswan_conf(wr_buffer,wr_buffer_size);

Answer 1

Here's my shot:

#define DO_SOMETHING(buf, str) \
    do { \
        const char *in = str; \
        char *out = buf; \
        const size_t len = strlen(in); \
        strcpy(out, in); \
        write_strongswan_conf(out, len);
    } while (0)

What are the reasons behind writing the macro exactly this way?

1. The do {} while(0) is not strictly necessary, but that's the only way to have a multi-statement operation properly expanded inside a condition. For example, if (condition) DO_SOMETHING(...). See a more elaborate answer about this.

2. The reason for creating temporary variables is that macro arguments are evaluated every time they appear in the macro body. For example, calling the macro PRINT_ME_TWICE(intval) printf("%d\n", intval); printf("%d\n", intval); with PRINT_ME_TWICE(i++) will print different values beucase the intval argument is evaluated twice, thus executing the post-increment operator (++) twice. You can only guarantee the evaluation problem won't happen by creating temporary variables, assigning the arguments to them, and never mention the arguments again.

NOTE: I'd argue against the use of strcpy if you can't guarantee the output buffer has enough space to hold the resulting string (plus the NULL terminator).

UPDATE:

As mentioned by @Bill Lynch in comments, the version above may still put you in trouble if you have variables with the same names the macro uses internally. This is known as Variable Shadowing. Any good compiler should be able to issue a warning about that. GCC for instance warns you even compiling without -Wall or -Wextra. Currently, the best we can do for a macro is a mere attempt to minimize the risk of shadowing by prefixing the variable names in our macros. That may be a good practice, but it's not a real solution. This is how it would look like then:

#define DO_SOMETHING(buf, str) \
    do { \
        const char *DO_SOMETHING_in = str; \
        char *DO_SOMETHING_out = buf; \
        const size_t DO_SOMETHING_len = strlen(DO_SOMETHING_in); \
        strcpy(DO_SOMETHING_out, DO_SOMETHING_in); \
        write_strongswan_conf(DO_SOMETHING_out, DO_SOMETHING_len);
    } while (0)

A good advice would be to replace macros by proper functions whenever possible, avoiding all obscurities of macros.

Answer 2

It's pretty straight forward:

#define FOO(buf, str) \
    strcpy(buf, str); \
    write_strongswan_conf(buf, strlen(buf));

Note that the buffer size argument was superfluous, so it is not needed here.

Then refer to it via:

int main ()
{
    char wr_buffer[1000];

    FOO(wr_buffer, "STRONG SWAN CONFIGURATION FILE");
    FOO(wr_buffer, "THIS IS THE SECOND LINE");

    return(0);
}

If you really did need a local argument in your macro, you could define a block with a macro:

#define FOO(buf, str) \
    { \
        int bsize; \
        strcpy(buf, str); \
        bsize = strlen(buf); \
        write_strongswan_conf(buf, bsize); \
    }

EDIT: Thanks to comments from @jweyrich and @BillLynch, they point out the potential pitfalls with the above method, and it is preferred to render a code block as follows when in a macro:

#define FOO(buf, str) \
    do { \
        strcpy(out, str); \
        write_strongswan_conf(buf, strlen(str)); \
    } while (0)

The block structure has one other benefit, and that is if you want to use your macro in an a compound statement in C. If you take the first definition above and do this:

if ( some_check )
     FOO(wr_buffer, "STRONG SWAN CONFIGURATION FILE");

You wouldn't get the expected results since the if would only apply to the first of the three statements. Of course, one good solution is to always block your statements:

if ( some_check ) {
     FOO(wr_buffer, "STRONG SWAN CONFIGURATION FILE");
}

But you could also use the pattern do { ... } while (0) in the macro definition as shown in the second example above, and that would avoid the issue as well.

Note that it is sometimes important to parenthesize arguments in macro definitions. I did not do so above since it isn't likely an issue, but let's suppose you have a macro such as this:

#define MUL(X, Y)   (X * Y)

This looks innocent enough. Let's try it:

int a = 2;
int b = 3
int c;

c = MUL(a, b);

This generates:

c = (a * b);

Note that #defines are quite literal. The parentheses are included. The preprocessor is just doing a direct substitution of everything I defined. If my expression is more complex:

c = MUL(a + 2, b + 3);

Then I get:

c = (a + 2 * b + 3);

Wait... this is going to give me a result which is equivalent to a + (2*b) + 3 which is probably not what I wanted. So I define the macro more carefully with parentheses:

#define MUL(X, Y)  ((X) * (Y))

Now the above will expand to: c = ((a + 2) * (b + 3));.

By the way, if you get any strange behavior when using macros and/or other #defines, it can be handy to run just the preprocessor on your .c file which will expand all of the #defines without compilation. The output can be a bit unwieldy at first, but it's handy for identifying problems with macros that are complex. You can see exactly what it generated.

In Unix/Linux, you would just run the cpp command: cpp myfile.c > myfile.pre or whatever you want to call the file. If you're using Visual Studio, there's likely an option for generating the preprocessed file.

For the record, if it were me, I'd probably make this a function, not a macro. It certainly saves only a tiny percentage of execution timemaking something like this a macro.

As in all aspects of coding: it's important to think through and carefully about what you're doing. :)

Answer 3

In my opinion, there is no call for a macro for this code. Use an array instead:

#include <stdio.h>

void write_strongswan_conf(const char *buffer)
{
    FILE *fp = fopen("strongswan.txt", "a");
    if (fp != 0)
    {
        fprintf(fp, "%s\n", buffer);
        printf("Data Written: %s\n", buffer);
        fclose(fp);
    }
}

int main(void)
{
    static const char *conf_string[] =
    {
        "STRONG SWAN CONFIGURATION FILE",
        "THIS IS THE SECOND LINE",
    };

    for (int i = 0; i < sizeof(conf_string) / sizeof(conf_string[0]); i++)
        write_strongswan_conf(conf_string[i]);

    return(0);
}

I'm blithely assuming it is sensible to open and close the file each time a string is written. I'm not convinced that's the case, but the original code made the assumption. (Opening a file is fairly expensive; you don't do it gratuitously, and that feels a bit gratuitous to me.)

The code in the write_strongswan_conf() function uses fprintf() instead of fwrite() since you're just writing strings, and that allows you to avoid concatenating a newline to the buffer, and therefore reduces the number of copies made. In fact, you don't even need to know how long the strings are; fprintf() will do the counting for you.

There really isn't anywhere in the resulting code that benefits from being converted to a macro; there isn't any sufficiently large chunk of repetition. The initialized array of strings can take 25 lines as well as it does 2. The writing function can be used with other sources of data if you wish. You could read the lines from another file, or have the user type them, or the program generate them. It is not tied to 1000-character lines (give or take — 998 would be the maximum safe length of string in the original code) and could handle longer lines. It error checks the fopen() call and doesn't try writing with a null file pointer if the fopen() fails; that avoids a crash.