1

I'm studying the coreutils source codes to get better in programming and I found these lines in base64.c and in others:

while ((opt = getopt_long (argc, argv, "diw:", long_options, NULL)) != -1)
switch (opt)
{
// ... other stuff
  case_GETOPT_HELP_CHAR; // <- this especially

  case_GETOPT_VERSION_CHAR (PROGRAM_NAME, AUTHORS);
// .. other stuff again

I didn't know what the heck this means, until I found this one in system.h :

#define case_GETOPT_HELP_CHAR           \
 case GETOPT_HELP_CHAR:         \
 usage (EXIT_SUCCESS);          \
 break;

I didn't know that you can actually make Macros that consists of so many statements! Isn't it risky to use so many statements in Macros or is this a good coding-style I should learn?

EDIT: Also I noticed there are actually alot of MACROS used in coreutils. It confuses me a little bit, because I come from a C++ background.

#define STREQ(a, b) (strcmp (a, b) == 0)

For example this one above, is it really necessary? It makes reading the code harder and there is not much saved by just doing STREQ in an inf statement

EDIT2: On the other hand, I like this one very much, thanks to Jay:

 #define COMMAND(NAME)  { #NAME, NAME ## _command }

 struct command commands[] =
 {
   COMMAND (quit),
   COMMAND (help),
   ...
 };

4 Answers4

1

There is no risk in using macros like that as long as they are properly written (if they aren't, the might result in compile error or unexpected behaviour). However, avoid them unless you need them - they tend to make code hard-to-read.

For example, a macro #define foo() bar(); bar would be dangerous since if(...) foo(); would end up with bar() always being called (in that case you would wrap the macro code in a do{ ... }while(0))

ThiefMaster
  • 310,957
  • 84
  • 592
  • 636
1

It depends. If the macros considerably reduce the code size, or make it considerably easier to modify things in just one place, by all means, go for it, even if it may sometimes look ugly to the eyes of a code poet.

In the particular case, I'm doubtful. First, the macro ends with a semicolon and the macro usage does as well, so the expansion ends with to semicolons, one of them a null statement. Some lints warn about those. The canonical way to force a semicolon after multi-statement macros is

#define FOO(x) do { statement(x); stmnt; } while (0)

but this fails in this case due to the, well, case. (Accidental pun, hehe).

Second, I'm not sure if this macro is actually reused somewhere. If not, then I'd consider it more code obfuscation than elegant hackery. On the other hand, it looks like system.h is included by other utilities as well, and there is a point in having consistency across a multitude of coreutils utilities, e.g. always using the same option char for "help", "verbose" etc.

Jens
  • 69,818
  • 15
  • 125
  • 179
  • `do{}while(0)` doesn't fail do the the `case`, because case labels in blocks are OK. It breaks because the `break` breaks the wrong loop. Sorry about your pun, but I tried to compensate... – ugoren May 15 '12 at 18:38
0

It's not risky and there is nothing wrong in it.

Please read here and use Google to get more idea about macros. Once you get some good understanding about macros, you will be able to decide when to use what.

Check this stackoverflow link also. : ) .

Community
  • 1
  • 1
Jay
  • 24,173
  • 25
  • 93
  • 141
  • 1
    Well, unless you're very careful about the macro and it's usage it's easy to get though-to-debug errors. – orlp May 15 '12 at 18:04
0

As a quick read through the answers would show you, it's a matter of preference. Some like it, some don't.
I don't.

My main problem with it, is that code using it isn't really C code. So when you read it, and you know C, you still can't understand it.

Such definitions, if used improperly, can lead to strange and hard to debug problems. You look at the code, and it looks right, but it isn't. The case_GETOPT_HELP_CHAR macro doesn't seem to be very prone to such errors. case_GETOPT_VERSION_CHAR, which has parameters, is likely to be more dangerous.

STREQ is a much better macro. It makes things a bit more clear than strcmp(a,b)==0 (I always find this one confusing).
As a rule of thumb, I prefer macros that could have been a function. You could implement STREQ as a function if you wanted, but not case_GETOPT_HELP_CHAR.

COMMAND is a different story. There's something ugly about it, but a strong justification - it removes repetition. Without macro tricks, you'd have to repeat the command name twice.
On the other hand, consider the poor guy who sees the quit_command function, and tries to find where it's called from. He can search for this name across the sources, and he won't find it.

ugoren
  • 16,023
  • 3
  • 35
  • 65