Search code examples
carraysstructmallocdynamic-memory-allocation

Accessing char array inside struct showing out of bounds error


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.


Solution

  • 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.