Difficulty printing char pointer array

Question

I've been struggling on this problem all day now, and looking at similar examples hasn't gotten me too far, so I'm hoping you can help! I'm working on the programming assignment 1 at the end of CH 3 of Operating Systems Concepts if anyone wanted to know the context.

So the problem is to essentially create a command prompt in c that allows users to input a command, fork and execute it, and save the command in history. The user can enter the command 'history' to see the 10 most recent commands printed out. The book instructed me to store the current command as a char pointer array of arguments, and I would execute the current one using execvp(args[0], args). My professor added other requirements to this, so having each argument individually accessible like this will be useful for those parts as well.

I decided to store the history of commands in a similar fashion using an array of char pointers. So for example if the first command was ls -la and the second command entered was cd.. we would have history[0] = "ls -la" and history[1] = "cd..". I'm really struggling getting this to work, and I'm fairly certain I'm screwing up pointers somewhere, but I just can't figure it out.

In main I can print the first word in the first command (so just ls for ls -la) using arg_history[0] but really can't figure out printing the whole thing. But I know the data's there and I verify it when I add it in (via add_history function) and it's correct! Even worse when I pass it to the get_history function made for printing the history, it prints a bunch of gibberish. I would greatly appreciate any help in understanding why it's doing this! Right now I have a hunch it's something to do with passing pointers incorrectly between functions, but based on what I've been looking at I can't spot the problem!

/**
 * Simple shell interface program.
 *
 * Operating System Concepts - Ninth Edition
 * Copyright John Wiley & Sons - 2013
 */

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



#define MAX_LINE        80 /* 80 chars per line, per command */
#define HIST_LENGTH     10

void get_input(char *args[], int *num_args, char *history[], int *hist_index);
void get_history(char *history[], int hist_index);
void add_history(char *history[], char *added_command, int *hist_index);

int main(void)
{
    char *args[MAX_LINE/2 + 1]; /* command line (of 80) has max of 40 arguments */
    char *arg_history[HIST_LENGTH];

    int num_args;
    int hist_index;

    int should_run = 1;
    int i;

    while (should_run){   
        printf("osh>");
        fflush(stdout);

        get_input(args, &num_args, arg_history, &hist_index);

        //printf("%s\n", arg_history[0]); //incorrectly prints up to the first space
        //printf("%s\n", args[0]) //prints the correct arg from the last command (eg. for 'ls -la' it prints ls for args[0] and -la for args[1])

        if (strcmp(args[0], "history") == 0) {
            get_history(arg_history, hist_index);
        }        
    }

    return 0;
}

void get_input(char *args[], int *num_args, char *history[], int *hist_index) {
    char input[MAX_LINE];
    char *arg;

    fgets(input, MAX_LINE, stdin);
    input[strlen(input) - 1] = NULL; // To remove new line character - the compiler doesn't like how I'm doing this

    add_history(history, input, hist_index);

    arg = strtok(input, " ");

    *num_args = 0;
    while(arg != NULL) {
        args[*num_args] = arg;
        *num_args = *num_args + 1;
        arg = strtok(NULL, " ");
    }
}

void get_history(char *history[], int hist_index) {
    int i;
    for (i = 0; i < HIST_LENGTH; i++) {
        printf("%d %s\n",  hist_index, *history);
        // prints gibberish
        hist_index = hist_index - 1;
        if (hist_index < 1) {
            break;
        }
    }
}

void add_history(char *history[], char *added_command, int *hist_index) {
    int i;
    for (i = HIST_LENGTH-1; i > 0; i--) {
            history[i] = history[i-1];
    }

    history[0] = added_command;
    *hist_index = *hist_index + 1;
    //printf("%s\n", history[0]); prints correctly
}

---

Update: I made the changes suggested by some of the solutions including moving the pointer to input out of the function (I put it in main) and using strcpy for the add_history function. The reason I was having an issue using this earlier was because I'm rotating the items 'up' through the array, but I was accessing uninitialized locations before history was full with all 10 elements. While I was now able to print the arg_history[0] from main, I was still having problems printing anything else (eg. arg_history[1]). But more importantly, I couldn't print from the get_historyfunction which is what I actually needed to solve. After closer inspection I realized hist_index is never given a value before it's used to access the array. Thanks for the help everyone.

Answer 1

input[strlen(input) - 1] = NULL; // To remove new line character - the compiler doesn't like how I'm doing this

Of course it doesn't. There are many things wrong with this. Imagine if strlen(input) is 0, for example, then strlen(input) - 1 is -1, and you're accessing the -1th item of the array... not to mention NULL is a pointer, not a character value. You probably meant input[strlen(input) - 1] = '\0';, but a safer solution would be:

input[strcspn(input, "\n")] = '\0';

---

history[0] = added_command;
*hist_index = *hist_index + 1;
//printf("%s\n", history[0]); prints correctly

This prints correctly because the pointer value added_command, which you assign to history[0] and which points into input in get_command is still alive. Once get_command returns, the object that pointer points to no longer exists, and so the history[0] pointer also doesn't exist.

You should know you need to use strcpy to assign strings by now, if you're reading a book (such as K&R2E). Before you do that, you need to create a new object of suitable size (e.g. using malloc)...

This is a common problem for people who aren't reading a book... Which book are you reading?

---

    printf("%d %s\n",  hist_index, *history);
    // prints gibberish

Well, yes, it prints gibberish because the object that *history once pointed to, before get_command returned, was destroyed when get_command returned. A book would teach you this.

See also Returning an array using C for a similar explanation...

Answer 2

Here are some description of strtok(). Because you just put the pointer of input to your history list instead of putting a copy, you'd only print out the first word.

char *strtok(char *str, const char *delim)
Parameters
str -- The contents of this string are modified and broken into smaller strings (tokens).

delim -- This is the C string containing the delimiters. These may vary from one call to another.