Segmentation Fault - Custom Shell

Question

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

#define BUF 1024        //I assume that the maximum number of arguments is 1024

main()
{
    char c;
    char *temp;
    char *arg[BUF];                 //the commands
    int i=1,j,k,iter=0;

    while(1)
    {
            i=1;
            iter=0;
            printf("CS21> ");
            temp = malloc(sizeof(char));
            while((c=fgetc(stdin))!='\n')
            {
                    temp = realloc(temp, i*sizeof(char));

                    temp[i-1]=c;
                    i++;
            }

            j=0;
            while(j<strlen(temp))
            {
                    if(temp[j]==' ')
                    {
                            j++;
                            continue;
                    }

                    if(temp[j]!=' ')  //Line 38: Same check performed as Line 42
                    {
                                    k=j;
                                    arg[iter] = malloc(sizeof(char));
                                    while(temp[k]!=' ')    //Line 42: Segmentation Fault here
                                    {
                                            arg[iter] = realloc(arg[iter],(k-j+1)*sizeof(char));
                                            arg[iter][k-j]=temp[k];
                                            k++;
                                    }
                                    iter++;
                                    k++;
                                    j=k;
                                    continue;
                    }
            }
    }
}

Hi, The above is a sample of code from my custom shell's code. I haven't completed the code yet, just in case you're wondering about the program going on till infinity. Now, I am getting a segmentation fault at a line (its been commented), but I don't understand why. I perform the same check as Line 42 at Line 38, but it didn't give a segmentation fault there. Can anyone help me out?

The purpose of some of the mentioned variables is as follows: "temp" is a pointer to a memory location that holds the entire command given to the shell. "args" is an array of pointers, each pointer pointing to a memory location that contains the individual arguments in the command.

For example, "temp" will hold the string - gcc hello.c -o hello, if that has been passed to my shell. And args[0] will point to "gcc", args[1] will point to "hello.c" and so on.

That is the purpose of this sample of code. It will store all the arguments in "args" after eliminating the white spaces from "temp". The while(1) loop will exit when the person calls the exit command from the shell. But that part of it will be done separately. Now can anyone help me with this sample of code?

Thanks!

Evidently you are accessing beyond the bound of the array. Do some debugging to check the values of the indices. — Oliver Charlesworth, Mar 03 '13 at 11:04
One hint: You don't terminate the `temp` string. Or the `arg` strings. — Some programmer dude, Mar 03 '13 at 11:05
I actually used gdb to isolate the line. But you haven't answered the question - How could it accept a line and then go on to give a segmentation fault at an exactly similar line (logically similar)? — , Mar 03 '13 at 11:07
Thank you for your time everyone! I actually didn't have to do anything relating to '\0' termination. I just hadn't checked the bounds, as V-X pointed out. — , Mar 03 '13 at 11:29
This is a very bad practice. You should ALWAYS add the null termination char! — Maroun, Mar 03 '13 at 12:14

score 1 · Accepted Answer · answered Mar 03 '13 at 11:08

1

You have loop in while(temp[k]!=' ') which doesn't finish, when there is no space in the string (case of last argument). You need to stop the loop if k > strlen(temp).

Just my comment: Who the hell is teaching to read by bytes and realloc after each character? This is awkward...

answered Mar 03 '13 at 11:08

V-X

2,979
18
28

Thank you V-X! Your suggestion worked! BTW, can you tell me a better way to read the commands and perform the same operation? – Mar 03 '13 at 11:26
`char* buffer; buffer = (char*)malloc(1024); fgets(buffer, sizeof(buffer), stdin);` you may check, if the string ends with newline and if the input is longer, then you can realloc the buffer and read another part. – V-X Mar 03 '13 at 12:25

Mikhail Vladimirov · Answer 2 · 2013-03-03T11:18:18.360

I think the following line:

arg[iter] = malloc(sizeof(char));

can damage value of k if item is greater than BUF. Also it can damage value of temp if iter is negative. This is because k and temp are stored on stack somewhat near arg and writing arg elements beyond its size may actually overwrite variables stored nearby.

Try printing k and temp before and after the line mentioned above and see whether the values of them are damaged.

score 0 · Answer 3 · answered Mar 03 '13 at 11:07

In the line

while((c=fgetc(stdin))!='\n')
{
        temp = realloc(temp, i*sizeof(char));

        temp[i-1]=c;
        i++;
}

you don't end the temp string with a null terminating character

and then later you go out of bounds of that array

 while(temp[k]!=' ')    //Line 42: Segmentation Fault here

Change the line

 temp[i-1]=c; to  temp[i-1]='\n';

and

while(temp[k]!=' ') to while(temp[k]!='\0')

score 0 · Answer 4 · edited May 23 '17 at 12:21

0

You need to allocate additional one char space for temp, for the special '\0' char which indicates end of string.

while((c=fgetc(stdin))!='\n')
{
    temp = realloc(temp, i*sizeof(char) + 1);  //1 more char space for '\0'
    temp[i-1]=c;
    i++;
 }
 temp[i] = '\0';  //Indicates the end of String

Not that the arg string has to end with '\0' too.

This will prevent the segmentation fault you are getting, but you might want to check another cases where your program might fall..

For more information, see this.

edited May 23 '17 at 12:21

Community

1
1

answered Mar 03 '13 at 11:20

Maroun

94,125
30
188
241

Well, it could crash if it never saw a '\n' and kept realloccing and assigned to the NULL pointer returned from the final realloc. – wildplasser Mar 03 '13 at 11:25

Segmentation Fault - Custom Shell

4 Answers4