0

I'm trying to input a string of characters to a pointer member of char type of an array of structure. The program is terminating after it receives the string for the member name of emp[0]. My code:

#include<stdio.h>
struct Employee
{
    char *name;
    int salary;
};

int main()
{
    struct Employee emp[3];

    int i;
    for(i=0;i<3;i++)
    {
        scanf("%s%d",emp[i].name,&emp[i].salary);
    }
    printf("\nOutput:");
    for(i=0;i<3;i++)
    {
        printf("\n%s%d",emp[i].name,emp[i].salary);
    }

    return 0;
}

When without array, the following code for some variable emp is working fine:

scanf("%s%d",emp.name,&emp.salary);

Any suggestions? Thanks in advance.

Nir Alfasi
  • 53,191
  • 11
  • 86
  • 129
PS Nayak
  • 409
  • 8
  • 20
  • 1
    `emp[i].name` isn't a valid pointer. Each one needs to be initialized (*e.g.*, with `malloc`). That needs to be done before you do your `scanf`, and could be done for each pointer within the same `for` loop. If it happens to work sometimes as it is now, you're just being lucky. It's not pointing to valid memory and it's randomly iffy whether it's in your program's allowed space. – lurker Nov 17 '17 at 17:46

4 Answers4

2

The name field in struct Employee is a pointer. You never give that pointer a value, but you pass it to scanf which then attempts to dereference it. Dereferencing an uninitialized pointer invokes undefined behavior.

Instead of using a char * for name, make it an array large enough to hold whatever value you expect:

struct Employee
{
    char name[50];
    int salary;
};
dbush
  • 205,898
  • 23
  • 218
  • 273
  • Yes, using array works fine. But I preferred pointer over array for flexibility in size. Thank you for your answer. – PS Nayak Nov 17 '17 at 18:26
  • 1
    If you want to use a pointer, you need to allocate space for it. That means you need to have some kind of idea how many characters the user will enter. If you have some way to know that before the user enters their name, you can use that. If not, then you have to use some hardcoded value, in which case you're better off using that value as an array size. – dbush Nov 17 '17 at 18:29
  • I used `malloc(3)` but the scanf() takes more than 3 characters. I could have limited the size by %3s but I will not since I do not know the size of a person's name. If I would have gone through array of size 3, it is definitely limiting the size. Is there a flaw in my understanding? – PS Nayak Nov 17 '17 at 18:41
  • 1
    @PSNayak If you don't limit the size of the user input in the `scanf` format, then you risk writing past the end of the array or allocated memory area. Your best bet is to use a large fixed size array as above to read into and limit the input to `scanf` to one less than the array size (to leave room for the null byte), i.e. `%49s`. – dbush Nov 17 '17 at 18:55
  • I got it clear now. It is dangerous not to put a limit to the input size. Thank you very much for the discussion. – PS Nayak Nov 18 '17 at 15:34
2

You need to initialize char pointer name before can point it to a String entered by user.

#include<stdio.h>
struct Employee
{
    char *name;
    int salary;
};

int main()
{
    struct Employee emp[3];

    int i;
    for(i=0;i<3;i++)
    {
        emp[i].name = (char*)malloc(3);
        scanf("%s%d",emp[i].name,&emp[i].salary);
    }
    printf("\nOutput:");
    for(i=0;i<3;i++)
    {
        printf("\n%s%d",emp[i].name,emp[i].salary);
    }

    return 0;
}
Yousaf
  • 27,861
  • 6
  • 44
  • 69
1

Or you can allocate something to name before passing it to scanf.

emp[i].name = malloc(sizeof*emp[i].name*MAX_LEN);
if( !emp[i].name )
// error in malloc.

What happens in your code?

scanf tries to write the characters read to the address contained by name. The address contained by name is indeterminate (some garbage value). Accessing it calls for undefined behavior.

When without array, the following code for some variable emp is working fine!

It works but it is an undefined behavior. The very next time you run the same code it may throw error.

What is that if after malloc?

In case malloc fails to provide with the requested memory it returns NULL. You are checking that to be sure that memory is allocated otherwise you won't access it because it invokes undefined behavior.

Is there anything else??

yes there are couple of things apart from all this

  • Don't cast the return type of malloc. It's unnecessary.
  • Free the dynamically allocated memory after you are done working with it. (using free()).
  • Check the scanf return value to be sure about whether the scanf call succedes.

Reply to user's comment:

If you have name[4] inside struct then you should write scanf("%3s",emp[i].name). This 3s limits the characters read by scanf to 3 avoiding buffer overflow. The thing is if you enter more than 3 characters you will read only 3 characters and rest of them will be in input stream.

Note: The question starts with an OR because the other way is what is answered by dbush. I didn't want to repeat it.

user2736738
  • 30,591
  • 5
  • 42
  • 56
0

By little improvement on others good answers, I want to introduce this version.

  1. We must always check return of malloc() to ensure success or its failure on allocate some desired memory location(s) on the Heap. If it fails, any try to access that unallocated memory cause Undefined Behavior.
  2. We must not cast return type of malloc() in C Why not cast return of malloc() its dangerous, see first answer.
  3. #include <stdlib.h> explicitly when using malloc().

Code

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

struct Employee
{
    char *name;
    int salary;
};

int main(){


struct Employee emp[3];

printf("Enter name, salary\n");

int i;
for(i=0;i<3;i++){
    emp[i].name = malloc(40*sizeof(char));

    if (emp[i].name != NULL){
                scanf("%s %d", emp[i].name, &emp[i].salary);
            }else{
                    ///If this accur, try to access inside below for loop is UB
                    printf("Cant allocate Memory for name[%d]\n", i);
            }
}

printf("\nOutput:\n");
for(i=0;i<3;i++)
{
    printf("%s: %d\n", emp[i].name, emp[i].salary);
}

///Free allocated memory 
printf("Free allocated memory\n");
for(i=0;i<3;i++)
{
    free(emp[i].name);
}

return 0;
}

Compile and Run

gcc -Wall so.c -o so && ./so

[Hint]

You must insert an space or hit enter, between Name and salary integer. Although this works but i recommend to separate scanf() for each section to get user input for pair of name, salary with hit enter key after each, this is readable, i think.

    scanf("%s", emp[i].name);
    scanf("%d", &emp[i].salary);

[Notes]

  1. Try to avoid using scanf(). Thanks for @Jonathan Leffler for providing this link that i steal from

  2. I recommend to take a look at scanf() leaves the new line char in the buffer, leaving a leading space when scan character " %c" inside scanf()

  3. malloc() documentation

[ToDo]

  1. Some Research on possible similarity between "%c", "%d" in scanf() behavior in scanning, remove buffer.
  2. provide some explain, links on Heap, Stack.
EsmaeelE
  • 2,331
  • 6
  • 22
  • 31