0

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.

chqrlie
  • 131,814
  • 10
  • 121
  • 189
  • Looking at the very small string lengths you are probably getting buffer overflow. Anyway, please read [Why is the gets function so dangerous that it should not be used?](https://stackoverflow.com/questions/1694036/why-is-the-gets-function-so-dangerous-that-it-should-not-be-used) The `gets()` function is obsolete. – Weather Vane Jun 22 '20 at 09:38
  • You are also calling `gets()` after `scanf()`. Please see [scanf() leaves the newline char in the buffer](https://stackoverflow.com/questions/5240789/scanf-leaves-the-new-line-char-in-the-buffer). – Weather Vane Jun 22 '20 at 09:40
  • I am calling `gets()` since I need to be able to take empty inputs as well which from posts here I see could be done by `gets()` or `fgets()` but not `scanf` – AROHAN AJIT Jun 22 '20 at 09:59
  • 1
    Please stop using `gets` *immediately*. Get all the input with `fgets()` and apply `sscanf` to extract the menu choice (or look at the first character). For strings you will need to [Removing trailing newline character from fgets() input](https://stackoverflow.com/questions/2693776/removing-trailing-newline-character-from-fgets-input/28462221#28462221) – Weather Vane Jun 22 '20 at 10:02

1 Answers1

2

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.

Adam
  • 2,820
  • 1
  • 13
  • 33
  • Thanks I completely missed that one. What I'm trying to do is have a dynamically allocated structure array which increases in length whenever user wants to add an entry. I'm keeping track of total entries using `count` – AROHAN AJIT Jun 22 '20 at 10:01
  • @AROHANAJIT - wellcome! i encourage you to go to man7.org in the link in my answer and keeps reading ! it will helps you trust me. finally, if my answer helped you please accept it (by pushing the grey V nearby the answer) and vote it up please. – Adam Jun 22 '20 at 10:07