0

In my code, I am trying to get strings as input from the user and store them in a pointer array. But, my code stores only the last entered string in all the elements of the pointer array. What modifications do I have to do?

My code is:


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

void main()
{
    char *names[ 4 ], name[ 10 ];

    for ( int i = 0; i < 4; i++ ) {
    
        printf( "Enter your name: " );
        scanf( "%s", name );
        names[ i ] = name;
    }

    for ( int i = 0; i < 4; i++ )
        printf( "\n* %s", names[ i ] );
}

The output is:

Enter your name: Anna Enter your name: Michelin Enter your name: Steven Enter your name: Jacob

  • Jacob
  • Jacob
  • Jacob
  • Jacob
CrazieGeek
  • 5
  • 2
  • 1
    `names[ i ] = name;` is not doing what you think. You need to use `strcpy` or similar. You also need to allocate memory for the strings in `names`. At the moment these are just 4 pointers, which you assign to the fixed address of `name`. You can change `names` to be a `char names[10][4];`. – wohlstad Jan 17 '23 at 08:05
  • 1
    @Fe2O3 oops. You are right - it should be `char names[4][10];`. – wohlstad Jan 17 '23 at 08:14
  • Argh! [`scanf( "%s", name )` is evil](https://stackoverflow.com/questions/2430303/disadvantages-of-scanf)! – Dúthomhas Jan 17 '23 at 08:42
  • Please do not use `void main()`! It is deprecated for ages, unless you are writing a OS kernel... – Serge Ballesta Jan 17 '23 at 08:59

1 Answers1

2

Shallow Vs deep copy:

Given two pointers p and q, the statement:

p = q;

doesn't copy the contents of the memory pointed to by q to the contents of the memory pointed to by p. It copies the pointer values, such that both p and q now point to the same memory, and any change to the memory via p is reflected when q is used. This is referred to as a shallow copy.

The statement

char *names[ 4 ];

declares an array of 4 pointers to char. NB that this only allocated memory for the pointers.

Then, in the loop:

for ( int i = 0; i < 4; i++ ) {
    
        printf( "Enter your name: " );
        scanf( "%s", name );
        names[ i ] = name;
}

You assign the address of the buffer name to each pointer in the array.

Question: What are the contents of name after the last iteration?

Answer: Jacob.

In memory, it'll look something like this:

names[0] ----
            |
names[1] ----
            |----> address of ```name```
names[2] ----
            |
names[3] ----

Dereferencing and printing the contents of those pointers, then, prints Jacob every time.

Possible fixes:

  • Use a 2-dimensional array as the comments suggest.

  • Allocate memory for the strings with malloc and assign their address to the pointers. Then you can use strcpy or POSIX's strdup to copy the contents of the buffer name to the memory region pointed to by each pointer (this is referred to as a deep copy).

Note: strdup uses malloc to allocate memory for the strings, so don't forget to free it.

The strdup() function returns a pointer to a new string which is a duplicate of the string s. Memory for the new string is obtained with malloc(3), and can be freed with free(3). (Linux's man page)

Incorrect definition of main()

From C11:

The function called at program startup is named main. The implementation declares no prototype for this function. It shall be defined with a return type of int and with no parameters:

int main (void) 
{ /* ... */ }

or with two parameters (referred to here as argc and argv, though any names may be used, as they are local to the function in which they are declared):

int main (int argc, char *argv[]) 
{ /* ... */ }

or equivalent; or in some other implementation-defined manner.

Buffer-overflow vulnerability:

scanf("%s", name);

is equivalent to:

gets(name);

It will happily continue to read input and overflow the buffer.

Instead, consider using fgets:

fgets (name, sizeof (name), stdin);

fgets reads at most n - 1 characters, and null-terminates the string.

Or use a width specifier with scanf:

scanf("%9s", name);
Harith
  • 4,663
  • 1
  • 5
  • 20