Accessing char array inside struct showing out of bounds error

1.1k views Asked by At

I have the following C struct and using the function getPerson(void) that returns a pointer to my struct returns a pointer to a new struct using user input. The following code does not compile and it gives the following error:

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

     typedef struct {

         char name[50];
         int age;

     } person;

     person* getPerson(void) 
     {   
         person* newPerson = (person*)malloc(sizeof(person));
         int ageInput;
         char *nameInputPtr = (char*)malloc(50 * sizeof(char));

         printf("Please enter your name: \n");
         scanf("%s", nameInputPtr);
         printf("Please enter your age: \n");
         scanf("%d", &ageInput);

         newPerson->name[50] = *nameInputPtr;
         newPerson->age = ageInput;

         return newPerson;  
   }

Error I get:

struct.c:22:2: error: array index 50 is past the end of the array (which contains 50 elements)
  [-Werror,-Warray-bounds]
    newPerson->name[50] = *nameInputPtr;
    ^               ~~
struct.c:6:2: note: array 'name' declared here
    char name[50];
    ^

I manage to fix my error by the following change in the line 22:

22  newPerson->name[49] = *nameInputPtr;

So my change was to change number 50, to number 49 to be inside the bounds of the index definition of line 6.

Therefore I don't understand why line 6 and line 22 give an error in my original code and I would like to have an explanation about the error and on the clarity and functionality of my solution to this.

3

There are 3 answers

0
Natasha Dutta On BEST ANSWER

Array index in C is 0 based. For an array with 50 bytes of memory allocated,

 char name[50];

trying to use [50] as index is off-by-one and invokes undefined behaviour.

That said,

 newPerson->name[50] = *nameInputPtr;

is not the way you copy a string. You need to make use of strcpy(), like

strcpy(newPerson->name, nameInputPtr);

Also, it is a good practice to limit the input string length while using scanf() to avoid possible buffer overrun. Change

scanf("%s", nameInputPtr);

to

scanf("%49s", nameInputPtr);

However, please keep in mind, there is not much point in using dynamic memory if you already have a design with fixed-sized allocation. You can vary easily make use of a compile-time allocated array.

3
LPs On

You have to use sprintf

strcpy(newPerson->name, nameInputPtr);

Take note that arrays in C are 0 based, the name[50] does not exist.

In your specific case you are using array as string, the you must verify that the size of your array is STR_LEN_MAX+1, because of strings are null terminated. That means strings always need a byte after last char in your string where a '\0' char can be inserted.

17
unwind On

What?

This:

newPerson->name[50] = *nameInputPtr;

says "assign the character at *nameInputPtr to the character at index 50 in name". But name is only 50 characters long, and arrays are 0-based in C so this is out of bounds.

Still, that code doesn't make any sense! You want:

strcpy(newPerson->name, nameInputPtr);

to copy the entire string. This runs the risk of propagating a buffer overrun since you don't limit the input in scanf(), though.

So, better, since you already have a person, just input into it:

scanf("%49s", person->name);

Remember to check the return value.

Of course you should do the same for the age, no need for a separate integer which is then copied into the structure.