Avoiding a large 'switch' call in the main loop

I am writing a Unix shell in C, which uses a rather large switch in the main loop.

It is used to handle built-in commands like 'cd' or'help'. The way it works is that it parses the user input and return a 'comand_t' struct, with 'name' being the first word of the input.

The in order to switch on a string, I translate it into an int by reading this initialized struct:

built_in_commands my_commands[CMD_NBR] = { 
    {""     , EMPTY_CMD         , ""},
    {"cd"   , CD_CMD            , "change directory"},
    {"env"  , ENV_CMD           , "print enviroment. Can show single variable"},
    {"exit" , EXIT_CMD          , "exit from AGROS"},
    {"help" , HELP_CMD          , "print help"},
    {"?"    , SHORTHELP_CMD     , "print short help"}
};

The macros are defined in the header file:

#define EMPTY_CMD           0

#define CD_CMD              1
#define ENV_CMD             2
#define EXIT_CMD            3
#define HELP_CMD            4
#define SHORTHELP_CMD       5
#define SETENV_CMD          6
#define GETENV_CMD          7

#define OTHER_CMD           8

#define CMD_NBR             9

the main loop of the program is this:

```
while (AG_TRUE){

/ Set the prompt /
get_prompt(prompt, MAX_LINE_LEN, username);

/*
* Read a line of input
* commandline should be deallocated with free()
*/
commandline = read_input (prompt);

parse_command (commandline, &cmd);

switch (get_cmd_code (cmd.name)){ //returns an int
case EMPTY_CMD:
break;

case CD_CMD:
change_directory (cmd.argv[1], ag_config.loglevel);
break;

case HELP_CMD:
print_help(&ag_config, cmd.argv[1]);
break;

case SHORTHELP_CMD:
print_help(&ag_config, "-s");
break;

case ENV_CMD:
print_env (cmd.argv[1]);
break;

case EXIT_CMD:
free (commandline);
commandline = (char *)NULL;

Function pointers

You want to use function pointers: http://www.newty.de/fpt/fpt.html

typedef void (*Command)(char const* args);  // Modify as required
                                            // But something like this
struct built_in_commands
{
    char const*   command;
    // int        commandID;   // Remove this you don't need it.
    char const*   man;
    Command       function;    // Add this: A pointer to a function
};

Then your switch becomes:

parse_command (commandline, &cmd);
if (cmd != NULL)
{    (*cmd->function)(commandline);      // Call the function associated with
}                                        // command (assuming you find it in the list)

Other Notes:

Basically, all of your function would have the same signature, and then be referred to inside of your struct instead of the number. Then you could call the function pointer from the struct and away you'd go.

case EXIT_CMD:
            free (commandline);
            commandline = (char *)NULL;
            closelog ();
            return 0;

This appears to be the only place you free commandline. But you allocate it everytime you get a command. This means that you have a leak. There actually isn't much of a point in freeing the memory here because it will all be free as soon as the process terminates anyways.

StackExchange Code Review Q#5386, answer score: 5