Storing using malloc() or realloc() and printing data in C

Question

Could someone please tell me about what is wrong with the code? Sorry if I'm new at this but I am trying to get this right for a long time. I am trying to get input from the user and then print the values. The following code is just an example.

I try to run the code but I get run-time errors. Can anyone pls help?

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

typedef struct poly
{
    int kill;
    float bill;
    char katana[50];
} koly;
 typedef koly* terma;

int main()
{
        int count = 0;
        terma ren;
        ren = (terma)malloc(sizeof(koly));
        ren = (terma)realloc(6*sizeof(koly));
        printf("We can store now:\n\n");
        while(++count<= 2)
        {
            scanf("%d",ren->kill);
            scanf("%f",ren->bill);
            scanf("%s",ren->katana);
        }
        while(++count<= 2)
        {
            printf("\n%d\n",ren->kill);
            printf("\n%f\n",ren->bill);
            printf("\n%s\n",ren->katana);
        }
}

Answer 1

About your error:

scanf("%d",ren->kill);
scanf("%f",ren->bill);

These should be pointers, so the corrected version is:

scanf("%d",&ren->kill);
scanf("%f",&ren->bill);

Also, realloc expects 2 arguments unlike malloc. The first should be a pointer to the address you are reallocing, and the second should be the new size. So:

ren = realloc(ren, 6 * sizeof(koly));

I have two other things to say about your code. First: it is unnecessary to cast the return value of malloc in C (and is frowned upon). Also, NEVER use scanf with the "%s" format specifier. This is extremely insecure and can easily lead to crashes and worse. The best method is to use fgets, so scanf("%s",ren->katana); would become this:

fgets(ren->katana, sizeof(ren->katana), stdin);

Edit: Here is my explanation for why casting the return value of malloc/realloc should be avoided:

It's not really because it's wrong; it's because it is bad practice to do so in C. Since a void* type is automatically converted to any pointer type, the typecasting is superfluous, the code is less readable (IMO), and it makes changes more difficult. For example, take your case:

ren = (terma)malloc(sizeof(koly));

I personally would rewrite this line like so:

ren = malloc(sizeof(*ren));

This allows for easily changing the type of ren. If ren were changed to another data type (such as int* or struct sockaddr_in* or anything else), it would automatically work. You wouldn't need to change the type in the cast or the type in the sizeof(). Both will automatically work for the new type. For arrays, it works the same:

int* myArr = malloc(42 * sizeof(*myArr));

If I instead wanted myArr to hold an array of 42 floats, the change is simple:

float* myArr = malloc(42 * sizeof(*myArr));

Note: In C++, the typecast is required as it doesn't automatically convert void* to other pointer types, but you really shouldn't be using malloc et al in C++ anyways when you have new (plus this is a C question, not C++).