(C) Print an array of structs from a function that calls other function

Question

I cooked a code for a program that asks the user to fill in information about a person (ReadDate/ReadPerson), then it asks for the range of people the user wants to print, and the function prints these people. (PrintRange)

I don't understand why the program is not working; it's stuck on the point when it should print the person... it's means there is probably a problem in either PrintPerson or PrintDate, or in the way I am calling these functions.

Here is my code:

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

typedef struct{
    int day, month, year;
}  Date;

typedef struct{
    char *first_name, *last_name;
    int id;
    Date birthday;
}  Person;

void ReadDate(Date *a)
{
    printf("Enter the day: ");
    scanf("%d", &a->day);
    printf("Enter the month (1-12): ");
    scanf("%d", &a->month);
    printf("Enter the year: ");
    scanf("%d", &a->year);
}

void ReadPerson(Person *b)
{
    char temp_first_name[21];
    char temp_last_name[21];

    printf("Enter the first name: ");
    gets(temp_first_name);
    b->first_name = (char*)malloc(strlen(temp_first_name)+1);
    strcpy(b->first_name,temp_first_name);
    //need to check malloc (later)

    printf("Enter the last name: ");
    gets(temp_last_name);
    b->last_name = (char*)malloc(strlen(temp_last_name)+1);
    strcpy(b->last_name, temp_last_name);
    //need to check malloc (later)

    printf("Enter the id number: ");
    scanf("%d",&b->id);

    printf("Enter the birthday:\n");
    ReadDate(&b->birthday);
}

int ReadAllDate (Person *b)
{
    //Person* b;
    int count=1;
    char choice;
    printf("Would you like to enter a person?(y,n)\n");
    choice = getchar();
    while(choice == 'y')
    {
        b = (Person*)malloc(1 * sizeof(Person));
        getchar();
        ReadPerson(b);
        printf("Done!\n");
        count++;
        getchar();
        printf("Would you like to add a person?(y,n)\n");
        choice = getchar();
    }
    count--;
    printf("The number of entered persons is %d\nBye\n", count);

    return count;
}

void PrintPerson(Person b)
{
    printf("%s  %s  %d\n", b.first_name, b.last_name, b.id);
}

void PrintDate(Date a)
{
    printf("%d // %d // %d\n",a.day,a.month,a.year);
}

void PrintRange(Person *b,int count,int ind1,int ind2)
{
    int i;

    if (ind1<0 || ind1>ind2 || ind2>count)
    {
        printf("error! you slip out from the limits of the array.\n");
    }
    else
    {
        for (i=ind1; i<=ind2; i++)
        {
            PrintPerson(*(b+i));
            PrintDate((b+i)->birthday);
        }
    }
}

int main(int argc, const char * argv[])
{
    Person* b;
    int count;
    int ind1, ind2;

    count = ReadAllDate(b);

    printf("insert the first index (the smaller): ");
    scanf("%d", &ind1);
    printf("insert the second index (the bigger): ");
    scanf("%d", &ind2);
    PrintRange(b, count, ind1, ind2);
}

"EXC_Bad_Access ... code..." its on the Xcode. (mac) .. its working for u in ur pc? — Michael Astahov, Jan 28 '17 at 17:50
ALWAYS, always *validate ALL user input* (e.g. `if (scanf("%d", &a->day) != 1) { /* handle error */ }` at minimum. — David C. Rankin, Jan 28 '17 at 19:17

G. Sliepen · Accepted Answer · 2017-01-28T18:06:31.617

2

The problem is that nowhere you are actually creating an array of Person objects. The function ReadAllDate() never changes the value of the variable b in main(). Instead, for every person you read in, it allocates memory for it, and stores the pointer to that Person in the local variable b in ReadAllDate(). Every time it does it, it forgets the previous value of b.

In main(), the value of b is unitinialized all the time. When you are trying to print persons, it will most likely crash or print garbage.

I see other issues with your code:

Don't use gets(), always use fgets(). The former can write past the end of the buffer you are providing.
Don't do malloc(strlen(...))+strcpy(), use strdup().
Start with count = 0, that way you don't need the count-- in ReadAllDate(). As a computer programmer, it's best to count from zero instead of one.
Write b[i] instead of *(b+i), and b[i].birthday instead of (b+i)->birthday.

edited Jan 28 '17 at 18:06

answered Jan 28 '17 at 18:00

G. Sliepen

7,637
1
15
31

Also the memory allocations for first_name and last_name should be `sizeof(char)*strlen(temp_first/last_name)` right ? – mahesh Rao Jan 28 '17 at 18:19
so i should do realloc at the end of the while loop in the function ReadAllDate to make it array in the wanted size? – Michael Astahov Jan 28 '17 at 18:27
See: [**c - Why is strdup considered to be evil**](http://stackoverflow.com/questions/12984948/why-is-strdup-considered-to-be-evil) – David C. Rankin Jan 28 '17 at 19:19
@maheshRao: `sizeof(char)` is *guaranteed* to be 1. – G. Sliepen Jan 28 '17 at 19:55
@MichaelAstahov: yes, `realloc()`ing is a good option. You still need to remember to return the pointer at the end. – G. Sliepen Jan 28 '17 at 19:56
@DavidC.Rankin: I'd rather use `strdup()` and perhaps implement it yourself once if it doesn't exist on your platform, than to do the `malloc`/`strlen`/`strcpy` dance every time, which is just waiting for a mistake to happen. – G. Sliepen Jan 28 '17 at 19:58
So im realloc-ing the pointer b at the beginning of the while loop , but how i can return the pointer? Im already returning "count". Its doesnt "return" it automaticlly because its a pointer? :/ – Michael Astahov Jan 28 '17 at 20:00
@G.Sliepen - agreed, it is just good to make clear it is not part of C99, etc.. without it being provided as an extension. Roll your own ends up being a `strlen`, `malloc`, `memcpy` (or `snprintf`) (but note there are inefficiencies with `strncpy` and a buffer sized greater than the data to be copied being zeroed) – David C. Rankin Jan 29 '17 at 19:11

(C) Print an array of structs from a function that calls other function

1 Answers1