0

I'm implementing a history feature for a command line shell. I've implemented a circular array to hold to ten most recent commands. Each command is also labeled by an integer specifying which total command is. For Example, if 30 total commands were entered, the ten commands in the circular array would be numbered (30, 29, 28, 27,...,21).

If a user were to insert the command "r" followed by a number labeling one of the ten instructions then that instruction is supposed to run. I keep running into a seg fault when trying to ensure that a two word command is accepted properly. Can anyone help point out what the problem is.

int main(void)
{
    char inputBuffer[MAX_LINE]; /* buffer to hold the command entered */
    int background;             /* equals 1 if a command is followed by '&' */
    char *args[MAX_LINE/2+1];/* command line (of 80) has max of 40 arguments */

    int position, count, rnum = 0;
    char historyArray[10][MAX_LINE];
    char *holder[MAX_LINE]={0};

    while (1){            /* Program terminates normally inside setup */
        background = 0;
        printf("COMMAND->");
        fflush(0);

        setup(inputBuffer, args, &background);       /* get next command */

        position = (count % MOD_VAL);
        strcpy(historyArray[position],args[0]);

        if(!strcmp("rr",args[0]))
        {
            strcpy(historyArray[position],historyArray[((position-1)+MOD_VAL)%MOD_VAL]);
            printf("%i",count);
            printf("%c",'.');
            printf("%c",' ');
            printf("%s",historyArray[position]);
            printf("%c",'\n');
            strcpy(args[0],historyArray[position]);
        }

        else if(!strcmp("r",args[0])) //SEG FAULT OCCURING IN THIS ELSE-IF BLOCK!
        {
            //args[1] will hold given number
            printf("%c",'\n');
            printf("%s",args[0]);
            printf("%s",args[1]);
            printf("%s",args[2]);
            printf("%c",'\n'); //PRINT STATEMENTS FOR DEBUGGING

            strncpy(holder[0], args[2], MAX_LINE - 1); //SEG FAULT

            rnum = atoi(args[1]);
            strcpy(historyArray[position],historyArray[((position-(count-rnum))+MOD_VAL)%MOD_VAL]);
            strcpy(args[0],historyArray[position]); //CHANGES VALUES OF args[1], args[2]

            if(holder[0] != NULL)
            {
                strncpy(args[1],holder[0],MAX_LINE-1);
                args[2] = NULL;
            }
            else
            {
                args[1] = NULL;
            }

            printf("%c",'\n');
            printf("%s",args[0]);
            printf("%s",args[1]);
            printf("%s",args[2]);
            printf("%c",'\n');
        }

        else if(!(strcmp("h",args[0]))||!(strcmp("history",args[0])))
        {
            int counter = 0;
            while(counter < 10)
            {
                printf("%i",(count - counter));
                printf("%c",'.');
                printf("%c",' ');
                printf("%s", historyArray[((position - counter + MOD_VAL)%MOD_VAL)]);
                printf("%c",' ');
                printf("%c",'\n');
                counter ++;

                if(counter > count)
                    break;
            }
        }
        count++;

        pid_t pid1; //Initialize pid_t variable to hold process identifier
        pid1 = fork(); //Fork process and assign process identifier to "pid1"

        if (pid1 == 0) //Child process
        {
            //Child process executes the command specified by the user and
            //then quits.
            execvp(args[0], args);
            exit(0);
        }
        else //Parent process
        {
            if (background != 1)//Check for inclusion of '&' in command 
            {
                wait(NULL); //Wait for child process to finish executing
            }
        } 

        /* the steps are:
         (1) fork a child process using fork()
         (2) the child process will invoke execvp()
         (3) if background == 0, the parent will wait, 
         otherwise returns to the setup() function. */
    }
}

Any assistance is appreciated!

-MATT

Matt Koz
  • 67
  • 3
  • 10
  • Matt, can you fix the indentation? Also, the code starts with an `else if` which doesn't make much sense. – JohnnyHK Mar 08 '13 at 03:09
  • Implementation works correctly for all single word commands (ls). Trying to ensure that it is functional for input commands such as mkdir dirname. User input: r 1 dirname; where r = args[0], 1=args[1], dirname=args[2]. Need dirname to be in args[1] at end of else-if case! – Matt Koz Mar 08 '13 at 03:09
  • Sorry, the code is somewhat lengthy and I was unsure if posting it in its entirety was acceptable. – Matt Koz Mar 08 '13 at 03:10
  • 4
    Use the debugger to find out where it is crashing. Then rerun having a watch on that variable. – Ed Heal Mar 08 '13 at 03:11
  • 2
    @MattKoz - Just narrow the problem down and post a small self contained example that demonstrates the problem. – Ed Heal Mar 08 '13 at 03:12
  • I posted the entire body and labeled where the problem is occurring. The exact line where args[1], args[2] changes (why i'm trying to save args[2] content) is also labeled. I apologize for any formatting issues that still may be present. – Matt Koz Mar 08 '13 at 03:21

3 Answers3

3

Here your args is the array of character pointers.

But strcpy requires two arguments - that should be array or character pointer to which memory allocated by malloc

But your strcpy(historyArray[position],args[0]); takes one argument as character pointer which will not be accepted.

so you can either change the args[] to args[][] or args[0] = malloc(some_no), segfault will be removed.

Hitesh Menghani
  • 977
  • 1
  • 6
  • 14
  • There is no declaration like `args[]`. And `args[][]` won't allocate any memory to the pointers in question, nor won't it even compile. – alk Mar 08 '13 at 07:08
0

You note that the crash occurs on the line

else if(!strcmp("r",args[0]))

If I were you, I would load the core file in a debugger and see what the value of args[0] is when passed to strcmp().

I expect you have compiler warnings about type mismatch between char and char*. You declare args as char*. That means args[0] is a char, not a char*. To compare the single character, just use a character instead of strcmp():

else if ('r' != args[0])

Some notes on pitfalls with C's string handling:

  • strcmp() isn't safe with respect to array boundaries when its arguments are not correctly NUL-terminated
    • use strncmp() to provide a limit to the number of characters compared
  • although strncpy() guards against array boundaries, it does not guarantee to NUL-terminate the destination string
  • strcpy() does not respect array boundaries; it is your responsibility to ensure the destination array is large enough to receive the string being copied to it
dsh
  • 12,037
  • 3
  • 33
  • 51
  • The segmentation fault occurs within that else-if block. I believe the actual seg fault occurs on this line: **`strncpy(holder[0], args[2], MAX_LINE - 1);`** Thank you for the tips! – Matt Koz Mar 08 '13 at 04:05
  • "... You declare args as char*. That means args[0] is a char, not a char* ..." at least from the OP's current version this is wrong. `args` is declared as `(char*)[]` which leads to `args[0]` being a `char *`, which is perfectly ok to be passed to `strcmp()`. And in fact `('r' != args[0])` whould lead to a "type mismatch" warning by the compiler. – alk Mar 08 '13 at 06:54
  • Right - my mistake. I missed the extra array dimension after the variable name. I like Java's syntax putting the array brackets on the type instead of the variable :). – dsh Mar 08 '13 at 15:41
-1

You are missing to allocated memory the the char pointers held in args and holder.

So referring to those as pointers to 0-terminated character arrays ("strings") via the str*() family of functions, leads to undefined bahaviour, as the str*() function try to derefernce those pointers not point to valid memory.

alk
  • 69,737
  • 10
  • 105
  • 255