Help implementing GNU Readline in C

Question

I am currently working on implementing a Unix shell. I am looking to use GNU Readline in order to enhance the interface. However, my code is generating random Segfaults, even during user input.

I stripped down the code to the maximum for readability:

agros.h

#define MAX_LINE_LEN 256
#define MAX_ARGS (MAX_LINE_LEN/2)
#define WHITESPACE " \t\n"

#define OTHER_CMD   0
#define EMPTY_CMD   1
#define EXIT_CMD    2

#define CMD_NBR     3


/*
 * This structure allows me to handle built-in commands with a switch statement
 * instead of multiple ifs. Since I cannot switch on a string, I associate a
 * command_code to each built-in command. The codes are define as preprocessor
 * instructions.
 */

typedef struct built_in_commands built_in_commands;
struct built_in_commands{
    char command_name[MAX_LINE_LEN];
    int command_code;
};

/*
 * This structure holds user input. 3 fields:
 *   - argv: an array of strings. Each word of the input is a case of the array.
 *   - name: the name of the executable called. By default it's argv[0]
 *   - argc: the number of words given in input. It's equivalent to the length of argv.
 *
 * Note to self: Some filenames have a space character (" ") in their names. I will have
 * to deal with that properly ... someday.
 */

typedef struct command_t command_t;
struct command_t{       
    char* name;
    int argc;
    char* argv[MAX_ARGS+1];
};



/*
 * These are the functions called by AGROS. These declarations are pretty explicit.
 * More detailed comments can be found in source files.
 *
 */

void    parse_command       (char *cmdline, command_t *cmd);
void    get_prompt          (char** prompt, char* username);
void    change_directory    (char* path);
int     get_cmd_code        (char* cmd_name);
c    har*   AG_Readline         (char* prompt);
void    set_homedir         (char** homedir);

agros.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <readline/readline.h>
#include <readline/history.h>
#include <pwd.h>
#include <sys/types.h>
#include "agros.h"


/*
 * A global array holding the associations between each built-in command
 * and their command_code. More explanations can be found in the declaration
 * of the "built_in_commands" structure.
 *
 * Notice how I "cheat" the dynamic allocation. Maybe there's a better way
 * to do this?
 */

built_in_commands my_commands[CMD_NBR] = {
    {"exit" , EXIT_CMD  },
    {""     , EMPTY_CMD },
    {"cd"   , CD_CMD    }
};



/* This variable contains the environment. I use it in my "env" built-in
   function */
extern char** environ;

/*
 * This function parses a string and fills a command_t struct.
 * It uses the strtok() to split the string into tokens. Then it fills the argv
 * array with the tokens.
 *
 * After filling the array, it copies argv[0] as the cmd->name and cmd->argc
 * as the length of the array.
 *
 */

void parse_command (char *cmdline, command_t *cmd){
    int count = 0;
    char* word;

    word = strtok (cmdline, WHITESPACE);

    if (word == NULL) { word = ""; } // Fixes blank line bug

    while (word) {
        cmd->argv[count] = word;
        word = strtok (NULL, WHITESPACE);
        count++;
    }
    cmd->argv[count] = NULL;

    cmd->argc = count;
    cmd->name = (char *) malloc (strlen (cmd->argv[0])+1);
    strcpy (cmd->name, cmd->argv[0]);
}


/*
 * Uses chdir() to change current working directory. Then updates the environement,
 * with the new value of PWD.
 *
 * If path is NULL, the function sends the user to his home directory. It builds the path
 * in the temp string "home" then copies it into "path" before freeing "home".
 *
 */

void change_directory (char* path){

    /* If no arguments are given, go to $HOME directory */
    if (path == NULL)
        set_homedir (&path);

    if (chdir (path) == 0){
        getcwd (path, MAX_LINE_LEN);
        setenv ("PWD", path, 1);
    } else {
        fprintf (stderr, "%s: Could not change to such directory\n", path);
    }

}

/*
 * This function access the global array variable my_commands
 * and returns the command_code eauivalent to each command.
 *
 */

int get_cmd_code (char* cmd_name){
    int i = 0;
    for (i=0; i<CMD_NBR; i++){
        if (!strcmp (my_commands[i].command_name, cmd_name))
            return my_commands[i].command_code;
    }
    return OTHER_CMD;
}

/* A static variable for holding the line. */
static char *line = (char *)NULL;

c    har* AG_Readline (char* prompt){
    /* If the buffer has already been allocated,
       return the memory to the free pool. */
    if (line){
        free (line);
        line = (char *)NULL;
    }

    /* Get a line from the user. */
    line = readline (prompt);

    /* If the line has any text in it,
       save it on the history. */
    if (line && *line)
      add_history (line);

    return (line);
}

/*
 * Setting variables using getuid() and getpwuid()
 * More info on these functions can easily be found in man pages.
 *
 */

void set_homedir (char** phomedir){
    struct passwd *pwd = NULL;
    pwd = getpwuid (getuid());
    *phomedir = pwd->pw_dir;
}

and finally main.c

#include <stdio.h>
#include <stdlib.h>
#include <string.h>
#include <unistd.h>
#include <sys/wait.h>
#include "agros.h"

int main (int argc, char** argv, char** envp){
    int pid = 0;
    command_t cmd = {NULL, 0, {NULL}};
    char* commandline = NULL;

        while (1){
        commandline = AG_Readline ("$ ");
        parse_command (commandline, &cmd);

        switch (get_cmd_code (cmd.name)){
            case EMPTY_CMD:
                break;

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

            case EXIT_CMD:
                return 0;

            case OTHER_CMD:
                pid = vfork();
                if (pid == 0){
                    printf ("Hello\n");
                    _exit(EXIT_FAILURE);
                }else if (pid < 0){
                    fprintf (stderr, "Error! ... Negative PID. God knows what that means ...\n");
                }else {
                    wait (0);
                }
                break;
        }
    }

    return 0;
}

I hope it's okay to paste such a large portion of code. And thanks for whoever reads it. NB: Feel free to comment on other things, like my awkward switch statement in the main. Thanks ^^

Answer 1

This array is NULL filled:

built_in_commands my_commands[CMD_NBR] = {
    {"exit" , EXIT_CMD  },
    {""     , EMPTY_CMD },
    {"cd"   , CD_CMD    }
};

As the value of CMD_NBR is 5 it is the equivalent off:

my_commands[3] = { NULL, 0 };
my_commands[4] = { NULL, 0 };

Thus in the function:

int get_cmd_code (char* cmd_name)

When you do the comparison test:

if (!strcmp (my_commands[i].command_name, cmd_name))

If i > 2 then the above will crash.