C - realloc with array of structs

Question

I'm doing a array of structs to dynamic allocate a list of products but this works only for a few times (3~5 times) and then I got this error.

* Error in `./test': realloc(): invalid next size: 0x000055bc0b44f260 *

Here is my code, this is part of a work for college.

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

typedef struct {
    int cod;
    float price;
} Product;

int main () {

    FILE *fp;

    fp = fopen("products.txt", "r+");

    if(fp == NULL)
        fopen("products.txt", "w+");

    int control = 1;
    int choice;
    int tam = 0;
    int i;
    Product *p1;

    p1 = malloc(sizeof(Product));

    while(fread(&p1[0], sizeof(Product), 1, fp) != NULL){
        tam++;
    }

    if(tam > 1){
        rewind(fp);

        p1 = malloc((tam) * sizeof(*p1));

        for (int i = 0; i < tam; i++){
            fread(&p1[i], sizeof(Product), 1, fp);
        }
    }

    rewind(fp);

    do {
        printf("1 - Add product\n2 - Show all products\n3 - Exit\n-> ");
        scanf(" %d", &choice);

        switch (choice) {
            case 1:
                 if (tam == 0) {
                    //p1 = malloc(sizeof(Product));
                    printf("Digit the product code: ");
                    scanf(" %d", &p1[tam].cod);
                    printf("Digit the product price: ");
                    scanf(" %f", &p1[tam].price);
                    tam++;
                } else {
                    printf("***Realloqing: %d***\n", tam * sizeof(*p1));
                    p1 = (Product*)realloc(p1, (tam) * sizeof(Product));
                    for (i = tam; i > 0; i--) {
                        p1[i].cod = p1[i-1].cod;
                        p1[i].price = p1[i-1].price;
                    }
                    printf("Digit the product code: ");
                    scanf(" %d", &p1[0].cod);
                    printf("Digit the product price: ");
                    scanf(" %f", &p1[0].price);
                    tam++;
                }
            break;
            case 2:
                for (i = 0; i < tam; i++) {
                    printf("Product code: %d\nProduct price: %f\n", p1[i].cod, p1[i].price);
                }
            break;
            case 3:
                control = 0;
            break;
        }

    } while (control);

    for (int i = 0; i < tam; i++){
        fwrite(&p1[i], sizeof(Product), 1, fp);
    }

    fclose(fp);

    free(p1);

    return 0;
}

Answer 1

The problem is here:

    printf("***Realloqing: %d***\n", tam * sizeof(*p1));
    p1 = (Product*)realloc(p1, tam * sizeof(Product));

When tam is, say 1, then you realloc tam * sizeof(Product), but you need to realloc (tam + 1) * sizeof(Product), because now you need space for 2 products.

So this fixes the problem:

    printf("***Realloqing: %d***\n", (tam + 1) * sizeof(*p1));
    p1 = (Product*)realloc(p1, (tam + 1) * sizeof(Product));

Answer 2

This:

p1 = (Product*)realloc(p1, (tam) * sizeof(Product));

Violates the First Rule of Realloc, which is that you must not assign the result of realloc directly to the same variable which is passed as its first argument. When you do, if it fails, you have lost (leaked) the old pointer.

Second, you do not check the return values from various functions, such as malloc(), realloc(), and scanf(). This too is a cardinal sin.

If fixing all that still leaves you with a broken program, use valgrind.