Problems of History for my own Shell in C

Question

As a project, I have to make my own shell. I did it but I have some problems with the history feature.

Here's piece of my code:

int main(int argc, char* argv[])
{
    char saisie[300], cwd[1024];
    char* nom = getenv("USER");
    char* backup[MAXCMD];
    int boucle = 1, n = 0, i, u = 0, b = 0;

    for(i = 0; i < MAXCMD; i++) 
    {
        backup[i] = NULL;
    }
    for(i = 0; i < MAX_INPUT_SZ; i++)
    {
        saisie[i] = 0;
    }
    char* cmd[MAXPARAMS];    //MAXPARAMS is 20

    while( boucle == 1)
    {   
        printf("%s@sam:~ %s> ", nom, (getcwd(cwd, sizeof(cwd))));
        fgets(saisie,MAX_INPUT_SZ,stdin);
        printf("\n");
        split_input(saisie, cmd);

        free(backup[u]);
        backup[u] = strdup(saisie);
        u = (u + 1) % MAXCMD;

        b = switchcmd(cmd,backup,b,u);
        start(cmd,b);
        b = 0;                      //débloquage fonction start
    }
    return 0;
}

I print the history with this fonction:

int historique(char* backup[], int u)
{
    int i = u;
    int place = 1;
    do
    {
        if (backup[i])
        {
            printf("%4d:  %s\n", place, backup[i]);
            place++;
        }
        i = (i + 1) % MAXCMD;
    } while (i != u);
        return 0;   
}

B is used to block the execution fonction (start) when user enter "cd" or "history", because it will generate an error.

Here's the fonction triggered when user enters "cd", "history", or "exit":

int switchcmd(char** cmd,char** backup, int b,int u)

{
    int i, n = 3, switch_value = 0;
    char* error;
    char* listcmd[n];
    listcmd[0] = "cd";
    listcmd[1] = "exit";
    listcmd[2]  = "history";

for (i = 0; i < n; ++i)
{
    if(strcmp(cmd[0], listcmd[i]) == 0)
    {
        switch_value = i + 1;
        break;
    }
}

switch (switch_value)
{
    case 1:
        chdir(cmd[1]);
        b = 1;
        error = strerror(errno);
        if (*error != 0)
        {
            printf("sam: %s: %s\n", cmd[0], error);
        }
        break;

    case 2:

        printf("Bye bye\n");
        exit(0);

    case 3:
        historique((char**)backup,u);
        b = 1;
        break;
}
return b;
}

When I execute my shell, and enter these commands successively, they work. °i1

> clear    
> ls -a -l
> ls -a
> cd ..
> man chdir

Then "history" for printing the history, I have this : °i2

   1:  clear
   2:  ls
   3:  ls
   4:  cd
   5:  man
   6:  history

and I want this output, with all parameters: °i3

1:  clear
2:  ls -a -l
3:  ls -a
4:  cd ..
5:  man chdir
6:  history`

I dont know why, and I don't understand why strdup does not duplicate my cmd in backup at it should.

Any help please?

Answer 1

When the user command is store in ' saisie ', this command is duplicate and split in array of parameters. And I use ' cmd ' in the execution fonction, with execvp.

Then there is your big problem, cmd has a fixed length of 1, if you use that to stored the command arguments for execvp, then you can only store one thing: NULL.

You have two options:

1. Use a large fixed size, for example char *cmd[100] where you can store up to 99 arguments and no more. This is the easiest solution but it is not flexible enough. Although some systems have a limit on the number of arguemnts you can pass to a new process, I don't know if there is a limit for all systems, this and this might help you there.

2. Dynamically create an array of char pointers depending on the command line. This is more work but this is also the more flexible solution. Assuming that your command line does not have support for pipes (|) and redirections (<, <<, >, >>), then split_input could look like this:

   char **split_input(const char *cmd)
   {
       if(cmd == NULL)
           return NULL;
   
       char **argv = NULL, **tmp;
   
       char *line = strdup(cmd);
       if(line == NULL)
           return NULL;
   
       const char *delim = " \t\n";
   
       char *token = strtok(line, delim);
       if(token == NULL)
       {
           free(line);
           return NULL;
       }
   
       size_t len = 0;
   
       do {
           char *arg = strdup(token);
           if(arg == NULL)
           {
               free_argv(argv);
               free(line);
               return NULL;
           }
   
           tmp = realloc(argv, (len + 2) * sizeof *argv);
   
           if(tmp == NULL)
           {
               free_argv(argv);
               free(line);
               return NULL;
           }
   
           argv = tmp;
           argv[len++] = arg;
           argv[len] = NULL; // argv must be NULL terminated
       } while(token = strtok(NULL, delim));
   
       free(line);
       return argv;
   }
   
   void free_argv(char **argv)
   {
       if(argv == NULL)
           return;
   
       for(size_t i = 0; argv[i]; ++i)
           free(argv[i]);
       free(argv);
   }
   

   Now you can use it like this:

   while( boucle == 1)
   {   
       printf("%s@sam:~ %s> ", nom, (getcwd(cwd, sizeof(cwd))));
       fgets(saisie,MAX_INPUT_SZ,stdin);
       printf("\n");
       char **argv = split_input(saisie);
   
       if(argv == NULL)
       {
           fprintf(stderr, "Cannot split command line, not enough memory\n");
           continue;
       }
   
       free(backup[u]);
       backup[u] = strdup(argv[0]); // <-- passing argv[0], not argv
                                    // but perhaps what you really want
                                    // is strdup(saisie)
       u = (u + 1) % MAXCMD;
   
       b = switchcmd(argv,backup,b,u);
       start(argv,b);
       b = 0;                      //débloquage fonction start
       free_argv(argv);
   }
   

You are also doing

backup[u] = strdup(cmd);

but the problem is that cmd is an array of char pointers, strdup expects a const char*, you are passing the wrong type. It should be strdup(cmd[0]) or strdup(saisie) if you want to store the whole command.