CODEWITHSUNDEEP
carraysstringpointers
user2916280
user2916280

Reputation: 25

C Array of pointers does not print as expected, it replaces everything with last input

I am struggling to fix this problem for 2 days now, but nothing seems to work! I am making a shell in C, and I am trying to implement the history command( which will keep a history of all commands given by the user). This is a simplified version of my code (removed unnecessary code, and functions).

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

int main()
{
int doLoop = 1;
int i=0;
int c=0;
char givenCommand[100];
char *history[20];
char *newlinePos; /* pointer to the '\n' character in the input C string */

/* Print the header */
printf("Operating Systems Shell (Fall 2013)\n");
printf("Iacovos Hadjicosti\n");
printf("\n");

while(doLoop==1) /* Check if it should do the loop again */
{
    printf("CSC327>"); /* Print a new prompt line */
    fgets(givenCommand, sizeof(givenCommand), stdin); /* get input */

    newlinePos = strchr(givenCommand,'\n'); /* point newlinePos to the '\n' character */
    if(newlinePos!=NULL)
    {
        *newlinePos = '\0'; /* replace it with the null character */
    }

    if(strcmp(givenCommand,"exit")==0)
    {
        doLoop = 0; /* Do not do the loop again */
    }
    else if(strcmp(givenCommand,"history")==0)
    {

        for(i=0; i<c; i++)
        {
            printf("%d. %s\n", i+1, history[i]);
        }
    }
    else
    {
        if(strcmp(givenCommand,"")!=0) /* if input was not empty.. */
        {
            printf("Command not found!\n"); /* show wrong command message */
        }
    }       

    history[c] = givenCommand;
    c++;
}
return 0;
}

This gets the input, put it in givenCommand, check which command it was, and then put it in the history array. when the user give the "history" command, it should print all commands in the history array. Instead, it prints only the last command given, c times (c is the total number of commands given).

for example, if the user give input "Test1", and then second input "Test2", when he give the input "history" the third time, it will output the following:

1.Test2

2.Test2

Any opinions how to solve this? (I am using TCC to compile)

Upvotes: 2

Views: 144

Answers (3)

Gangadhar
Gangadhar

Reputation: 10516

Modify this part 

else if(strcmp(givenCommand,"")==0) /* if input was empty.. */
    {
        printf("Command not found!\n"); /* show wrong command message */
    }

else
    {

    history[c]=malloc(strlen(givenCommand)+1); //allocate memory

    //check malloc allocated memory or failed 

    if(history[c]==NULL)  
        {
        printf("malloc function failed \n"); 
        perror("ERROR");
        exit(EXIT_FAILURE);
        // exit(1); //if you don't want to exit then break loop with break;  
        // As alk suggested, The use of EXIT_SUCCESS and EXIT_FAILURE is 
        // slightly more portable (to non-UNIX environments)   
        // than the use of 0 and some nonzero value like  1  or  -1. 
        }  

    strcpy(history[c], givenCommand); // if you can able to use Unix platform you can also use this instead of allocating memory copyinghistory[c] =strdup( givenCommand );
    c++;
    }

Edited strdup() would not work on windows because it is POSIX specific.

what happens in your case

char givenCommand[100]; is static declaration address is same.

when you enter "test1"  

                    getcommandname starting address
                    test1
                    ^
                    | 
history[0]----------|


when you enter "test2"                          
                    getcommandname starting address
                    test2
                    ^
                    | 
history[0]----------|
history[1]----------| 


When you enter "history"                        
                   getcommandname starting address
                   history
                   ^
                   | 
history[0]---------|
history[1]---------| 
history[2]---------|

Upvotes: 2

Kninnug
Kninnug

Reputation: 8053

You need to copy givenCommand into the history, instead of assigning its pointer:

strcpy(history[c], givenCommand);

For this you must change history so it has space for the commands themselves:

char history[20][100];

As a stylistic note: you could put continue; at the end of the if-body for each command so you can omit the else part. For "exit" you can use break; to end the loop immediately:

if(strcmp(givenCommand, "exit") == 0){
    break; /* end the loop now */
}

if(strcmp(givenCommand, "history") == 0){
    /* do history stuff */
    continue; /* go back to the beginning of the loop */
}

Upvotes: 2

wye213
wye213

Reputation: 1

Because every time after the input givenCommand pointing to content are changing, so the content of the need to save the input, rather than save the pointer to the input.so may be you could use strcopy function to save the input string

Upvotes: 0

Related Questions