Search code examples
cpointersstructmallocdynamic-memory-allocation

Lots of garbage characters in struct pointer


I am new to C, and I was trying to make a phone book. The code for which is:

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

int count = 0;
struct Telephone {
    char fname[20];
    char lname[20];
    char countrycode[2];
    char areacode[3];
    char number[8];
};

void addRecord(struct Telephone *T) {
    if (count != 0) {
        struct Telephone *temp = (struct Telephone *)realloc(T, count + 1);
    }
    printf("Count: %d", count);
    printf("Enter first Name: ");
    getchar();
    do {
        gets(T[count].fname);
        T[count].fname[strlen(T[count].fname)] = '\0';
        if (strlen(T[count].fname) == 0)
            printf("Empty Value Not Permitted! Please try again: ");
    } while (strlen(T[count].fname) == 0);
    printf("Enter last name: ");
    gets(T[count].lname);
    printf("%s", T[count].lname);
    printf("Enter Country Code: ");
    gets(T[count].countrycode);
    printf("Enter Area Code: ");
    gets(T[count].areacode);
    printf("Enter Telephone Number: ");
    do {
        gets(T[count].number);
        T[count].number[strlen(T[count].number)] = '\0';
        if (strlen(T[count].number) == 0)
            printf("Value Not Permitted! Please try again: ");
    } while (strlen(T[count].number) == 0);
    count += 1;
    printf("Added\n");
}

void main() {
    struct Telephone *T = (struct Telephone *)malloc(sizeof(struct Telephone));
    int option = 1;
    while (option != 0) {
        printf("1 -> Add new record\n2 -> Search for record\n3 -> Display all record\n4 -> Update record\n0 -> Exit\nEnter: ");
        scanf("%d", &option);
        if (option == 1)
            addRecord(T);
        if (option == 3) {
            for (int i = 0; i < count; i++)
                printf("FN: %s LN: %s N: %s", T[i].fname, T[i].lname, T[i].number);
        }
    }
}

I only made add and display method at start to check if everything is added correctly. However when I try to add values, after first entry, the value stored is garbage. I tried to add 3 values and printed first, last name and number. The values I got are:

FirstName: hello LastName: └ Number: 123
FirstName: hello1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: Number: o1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: Number:
FirstName: ame: : hello1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: Number: o1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: Number:
 LastName:  LastName: : hello1 LastName: Number: o1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: Number:
 LastName:  LastName: : hello1  Number: ame: Number: o1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: Number:
 LastName:  LastName: : hello1 LastName: Number: o1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: : hello1 LastName: Number:
 LastName:  LastName: : hello1  Number:

For reference, inputs were: 1->hello,world,123 2-> hello1,world1,456 3->hello2,world2,789 Why is this garbage value printing and how to fix it. I have used dynamically allocated structure pointer here.


Solution

  • you have alot of things to fix in you code. but lets start from the most critical one. this is not the way we do reallocation in c.

    if (count!=0){
        struct Telephone* temp = (struct Telephone*)realloc(T,count+1);
    }
    

    what you have done in the above code, you have freed (realloc do for you) the T buffer and allocate count +1 bytes on the heap instead! This is what you want to do ??

    see realloc docs from man7. realloc

    void *realloc(void *ptr, size_t size);
    

    The realloc() function shall deallocate the old object pointed to by ptr and return a pointer to a new object that has the size specified by size ... continue reading

    how you should reallocate?

    if (count!= 1){
            struct Telephone* temp = (struct Telephone*)realloc(T, count*(sizeof(struct Telephone)));
            if(temp==NULL){ /*we always check the return vale of re/c/m/alloc() */
               .... do work ...
            }
        }
    

    ----------------------------------------------------- CAUTION ------------------------------------------------------------

    we dont reallocate a copy of the pointer recieved to a function (evrey thing in c passed by value)!

    so after reallocating the T pointer, in your function addRecord (life is good), realloc freed the previous allocated buffer pointed by T, and now the pointer of the callee is pointing to a released buffer !!! and he is not a ware of you reallocating! next time he tries to access the T pointer, its UB

    how we prevent this ?

    double pointer T**, and deref the callee pointer (realloc the original pointer *T) to point to the new allocated buffer!

    the callee calls addRecord like this:

    truct Telephone *T = malloc(...) 
    addRecord(&T)
    

    function signature become:

    void addRecord(struct Telephone **T){
        if (count!=0){
        struct Telephone* temp = (struct Telephone*)realloc(*T,count+1);
        }
        ....
    }
    

    -----------------------------------------------------------------------------------------------------------------------------------

    I dont know what you trying to achieve here! but start from fixing the realloc bugs.