newbieboyik
newbieboyik

Reputation: 39

Unable to add things into my array properly with c

int main(void){
    char name[8], comName[24];
    int numComponents, numSchemes, i;


    printf("\n\nHow many marking components in the course? ");
    scanf("%d", &numComponents);
    char *listComponents[numComponents];

    i=0;
    while (i<numComponents){
        printf("\tenter next component name: ");
        scanf("%s", name);
        listComponents[i] = name;
        i++;
    }

    printf("\nThis is name #1 =     %s", listComponents[0]);
    printf("\nThis is name #2 =     %s", listComponents[1]);
    printf("\nThis is name #3 =     %s", listComponents[2]);


}

I have this function which asks the user how many names numComponents there are, and then initializes an array of Strings that size *listComponents[numComponents].

I then iterate and ask the user for the input and then put it into the array as I go through. However I'm having a problem where I would input "Math" and then "English" and then "History.

But once I start printing it out to see what the value is, listComponents[0] [1] and [2] are all History.

I was wondering what is causing this and how I would be able to fix it? Am I writing into the array incorrectly, or trying to access the array incorrectly, or both?

Upvotes: 2

Views: 62

Answers (4)

user3629249
user3629249

Reputation: 16540

int main(void)
{
    char name[8]; = { '\0' }; // note: this may be too short for some component name
    char comName[24]; // the compiler should give a warning about unused variable
    int numComponents = 0; 
    int numSchemes; // the compiler should give a warning about unused variable
    int i = 0; // index/loop counter


    printf("\n\nHow many marking components in the course? ");
    if( 1 != scanf(" %d", &numComponents) ) // note: leading space in format string
    { // then scanf() failed
        perror( "reading number of components with scanf" );
        return( 1 );
    }

    // implied else

    char *listComponents[numComponents] = { NULL );

    for( i=0; i<numComponents; i++ )
    {
        printf("\tenter next component name: ");
        if( 1 != scanf(" %s", name) ) // note: leading space in format string
        { // then, scanf() failed
            perror( "reading component name with scanf" );
            return( i+1 ); // +1 so will not return good indication of 0
        }

        // implied else

        listComponents[i] = malloc( strlen(name) + 1 );
        if( NULL == listComponents[i] )
        {
            // free malloc'd areas in listComponents[]
            for( i=0; i< numComponents; i++ )
            {
                free(listComponents[i]); // note: calling free() with NULL is ok
            }
            perror( "malloc" );
            return( numComponents ); // return a positive value error code
        }

        // implied else

        strcpy( listComponents[i], name );
    }

    for( i=0; i<numComponents; i++ )
    {
        printf("\nThis is name #%d =     %s", i, listComponents[i]);
    }

    // free malloc'd areas in listComponents[]
    for( i=0; i< numComponents; i++ )
    {
         free(listComponents[i]);
    }

    return( 0 );
}

Upvotes: 0

utarid
utarid

Reputation: 1715

If you use char array, not char pointer, it will work

char name[8], comName[24];
int numComponents, numSchemes, i;
int MAX_CHAR = 10;

printf("\n\nHow many marking components in the course? ");
scanf("%d", &numComponents);
char listComponents[numComponents][MAX_CHAR];

i=0;
while (i<numComponents){
    printf("\tenter next component name: ");
    scanf("%s", name);
    strcpy(listComponents[i] , name);
    i++;
}

printf("\nThis is name #1 =     %s", listComponents[0]);
printf("\nThis is name #2 =     %s", listComponents[1]);
printf("\nThis is name #3 =     %s", listComponents[2]);

Upvotes: 0

Jimbo
Jimbo

Reputation: 4515

The problem looks like you are creating an array of character pointers and then setting each array element to point to the same buffer. Therefore each array element will print whatever the last value you wrote to the buffer.

If you want to have an array of N different inputs better to create N new strings and store pointers to the new strings. Have a look at, for example, strdup() (or maybe strndup() for the safety concious). Remember to free() allocated memory :)

For example

while (i<numComponents){
        printf("\tenter next component name: ");
        scanf("%s", name);
        listComponents[i] = strdup(name); //<---- pointer notice **copy** of buffer stored, not     pointer to buffer :)
...
...
<snip>
...
...

for(i=0; i<numComponents;++i) free(listCompoents[i]);

The way you do it you get

listComponents[0] ----- points to ------> name[]
listComponents[1] ----- points to ----/
listComponents[2] ----- points to ---/
...
listComponents[n] ----- points to -/

So, you can see they all point to the same buffer, or area of memory, so printing each will always yield, at any time, the string in name[]

Using strdup() you get

listComponents[0] ----- points to ------> new buffer in memory holding copy of name[]
listComponents[1] ----- points to ------> new buffer in memory holding copy of name[]
listComponents[2] ----- points to ------> new buffer in memory holding copy of name[]
...
listComponents[n] ----- points to ------> new buffer in memory holding copy of name[]

NOTE: When copying any user input, although I have used strdup() you may want to use strndup() to avoid large user input or user input that has overrun your name[] buffer. If you are malloc()ing a constant buffer size and then strcpy()ing you defo want to use strncpy() to avoid overflowing your allocated buffers

Upvotes: 4

Haris
Haris

Reputation: 12270

The problem is with this line

listComponents[i] = name;

you realize that name[] is just one array and everytime the loop iterates and reads the user input to name[] it is overwriting to the same address space (thats pointed by name).. moreover, every iteration assigns a new address to listComponents[i] its actually storing the same address of name[] at all the places

What you can do is use malloc() to achieve what you want, like

while (i<numComponents){
    printf("\tenter next component name: ");
    name = malloc(20);
    scanf("%s", name);
    listComponents[i] = name;
    i++;
}

and change the name declaration to char* name;

Upvotes: 3

Related Questions