Simple command line parsing

Question

I'm making a project that needs a simple cross-platform command-line parser. I made this. Hope you enjoy!

#include <stdio.h>
#include <string.h>
#include <stdlib.h>
#include <stdarg.h>
#define __NUM_OF_ELEMENTS(ARR) (sizeof(ARR)/sizeof(ARR[0]))
#define __HAS_ARG(FLAGS, ARG) (FLAGS & (1 << ARG))
void panic(const char *fmt, ...){
 fprintf(stderr, "error: ");
 va_list arglist;
 va_start( arglist, fmt );
 vfprintf(stderr, fmt, arglist);
 va_end( arglist );
 exit(EXIT_FAILURE);
}
void help(const char * progName){
 char * whoami = 0;
 (whoami = strrchr(progName, '/')) ? ++whoami : (whoami = progName);
 printf(
 "%s is intended to do blah\n"
 "Options:\n"
 " --help Display This message\n"
 " --file [file name] Input file to use\n",
 whoami);
 exit(EXIT_SUCCESS);
}
int main(int argc, char *argv[])
{
 if(argc == 1){
 help(argv[0]);
 }
 const char * args[] = {"--help", "--file", "--2cool4scool"};
 enum {HELP_ARG, FILE_ARG, TOO_COOL_FOR_SCHOOL_ARG};
 unsigned int flags = 0;
 //skip prog name
 for(unsigned int arg = 1, i = 0; arg < argc; ++arg){
 for(i = 0; i < (unsigned int)__NUM_OF_ELEMENTS(args); ++i){
 if(!strcmp(argv[arg], args[i])){
 flags |= 1 << i;
 goto CMD_ARGUMENT_FOUND;
 }
 }
 //argument is not found / unkown arg
 panic("Argument '%s' not found\n", argv[arg]);
 CMD_ARGUMENT_FOUND:;
 }
 if(__HAS_ARG(flags, HELP_ARG)){
 help(argv[0]);
 }
 if(__HAS_ARG(flags, TOO_COOL_FOR_SCHOOL_ARG)){
 puts("Hell YA!");
 }
 return 0;
}

Answer 1

Since this is a single-translation-unit program, mark your panic and help as static.

About this:

(whoami = strrchr(progName, '/')) ? ++whoami : (whoami = progName);

"Yikes". Generously we call this write-only code. Unroll it to

char *whoami = strrchr(progName, '/');
if (whoami)
 whoami++;
else
 whoami = progName;

In this loop, your i declaration should be moved down:

for(unsigned int arg = 1, i = 0; arg < argc; ++arg){
 for(i = 0; i < (unsigned int)__NUM_OF_ELEMENTS(args); ++i){

should be

for (unsigned int arg = 1; arg < argc; ++arg) {
 for (int i = 0; i < (unsigned int)__NUM_OF_ELEMENTS(args); ++i) {

to narrow its scope.

Don't use goto. Factor out a function where the double loop termination is done using a return.

• Alex Angel: Thx, I'll keep that in mind
• Alex Angel: A few things to things note is the compiler will emit a warning at for (int i = 0; i < (unsigned int) ... as it is comparing an int to an unsigned int; I had initially declared i in the 1st for loop as it would be technically faster, but as you pointed out it would be better to do it the way you stated for clarity; In this example I agree help should be static, but panic would probably be used later in the application. With whoami it does make more sense to avoid the one liner
• Reinderien: it would be technically faster - I deeply doubt this. I encourage you to profile to see whether that's the case.
• Reinderien: Perhaps you're working under the assumption that re-declaring a variable imposes a runtime cost. It would not in this case.
• Alex Angel: I had heard something a while ago, but it doesn't really make a difference in this case besides clarity.

Answer 2

These names are dangerous:

#define __NUM_OF_ELEMENTS(ARR) (sizeof(ARR)/sizeof(ARR[0]))
#define __HAS_ARG(FLAGS, ARG) (FLAGS & (1 << ARG))

Any identifier containing two consecutive underscores is reserved for the implementation for any purpose. The same goes for names beginning with underscore and a capital letter.

Also, you slightly messed up the protective parentheses when expanding macro arguments:

#define NUM_OF_ELEMENTS(ARR) (sizeof (ARR) / sizeof (ARR)[0])
#define HAS_ARG(FLAGS, ARG) ((FLAGS) & (1 << (ARG)))

---

If no arguments are passed, then we hit this:

if(argc == 1){
 help(argv[0]);
}

In a portable program, we should really test argc <= 1, since implementations are allowed to pass 0 as argc. The usage message should go to stderr, not stdout. And why do we continue after this? It seems very odd to print an unasked-for usage message, but then continue and return a success status.

---

Maintaining parallel lists is tedious and error-prone. So this code looks pretty straightforward with three possible options:

const char * args[] = {"--help", "--file", "--2cool4scool"};
enum {HELP_ARG, FILE_ARG, TOO_COOL_FOR_SCHOOL_ARG};

It becomes unmanageable once you have a few dozen options. And it doesn't handle synonyms very well (e.g. -f and --file both being acceptable).

flags |= 1 << i;

That ought to be 1u << i, since flags is unsigned (same goes for the corresponding macro). And this fails silently once i becomes greater than the width of unsigned, leading to a hard-to-find error.

---

 //argument is not found / unkown arg

Typo: unknown

---

if(__HAS_ARG(flags, HELP_ARG))
if(__HAS_ARG(flags, TOO_COOL_FOR_SCHOOL_ARG))

It seems that the --file option is recognised but ignored. That's almost certainly a bug. Worse than that, the help says it should be followed by a file-name argument, but actually doing so results in an "argument not found" error.

---

It's worth reading the documentation of Python's argparse module (even if you don't know Python) to see what a real argument handler can and should do. For example, it automates printing the usage message so you don't end up forgetting to update it when you add a new option, like here:

 "Options:\n"
 " --help Display This message\n"
 " --file [file name] Input file to use\n",

• Alex Angel: I am impressed! Although I knew a lot of compiler macros used __ at the beginning of the names I didn't really consider that an issue. I'll use #define lengthof(ARR) (sizeof (ARR) / sizeof (ARR)[0]) next time. I initially thought sizeof() was a macro until I saw that you had said (ARR)[0]. As for the --file arg being ignored that was intentional, but I do concede that I should have tested for it and then did printf("file: %s\n", filename)
• Alex Angel: As for the section where you say Maintaining parallel lists ... I do agree when it gets to more than 5 args. One solution in the meantime I'd say is to do to somewhat alleviate it is struct arg{const char* str; unsigned int id;}; and then struct arg arglist [] = {{"--help", HELP_ARG}, {"-h", HELP_ARG}}; So everything is in one place.
• Toby Speight: Yes, or even perhaps struct arg { const char *option; const char *help; enum ArgType type; void *storage;}, where ArgType determines what gets written to *storage.
• Alex Angel: Well, that's a little more simple. Where you say ... void *storage;}, where ArgType determines what gets written to *storage I don't understand; What would storage point to?
• Toby Speight: For a boolean flag, the caller would set it to point to a bool; for a string argument like --file, perhaps to a const char* (we can point that at one of the argv strings); for an integer argument, it would point to a int or unsigned int or long, etc. It's something you might want to experiment with - and come here for a review when you have something you're happy with!