My code consist in a aplication which receive by console one client (clientes
) and add it into an array created with malloc. After we can remove this cliente. Also we can add a new travel (viajes
) and remove it (process is analog to showed in code).
I am trying to remove last item of an array of struct created with malloc, as I said earlier. I will show you the code (how I add a new clientes
and how i try to delete it). Also say that errors I get are commented in the line which produce the error, so I think it is more visual. Also say the code is MCVE, so only copying code will run. The code is of my aplication is:
#include<stdlib.h>
#include<stdio.h>
#include<string.h>
// DEFINE STRUCTS
struct viaje {
char *identificador;
char *ciudadDestino;
char *hotel;
int numeroNoches;
char *tipoTransporte;
float precioAlojamiento;
float precioDesplazamiento;
};
struct cliente {
char *dni;
char *nombre;
char *apellidos;
char *direccion;
int totalViajes;
struct viaje *viajes;
};
int main(){
// Variables
int i = 0;
int totalClientes = 0;
char dni[255];
char nombre[255];
char apellidos[255];
char direccion[255];
// Init array of struct cliente (in var clientes)
struct cliente *clientes = (struct cliente *)malloc(0);
printf("Input number of elements of struct clientes: ");
scanf("%i", &totalClientes);
fflush(stdin);
// ADDING NEW ELEMENTS INTO CLIENTES
for (i = 0; i < totalClientes; i++) {
// Receive parameters
printf("Input NIF: ");
gets(dni);
fflush(stdin);
printf("Input NAME: ");
gets(nombre);
fflush(stdin);
printf("Input SURNAME: ");
gets(apellidos);
fflush(stdin);
printf("Input ADDRESS: ");
gets(direccion);
fflush(stdin);
// Create memory for his child (MAX_TAM_* are #define int)
clientes = (struct cliente *)realloc(clientes, (i+1)*sizeof(struct cliente));
clientes[i].dni = (char *)malloc(200*sizeof(char));
clientes[i].nombre = (char *)malloc(200*sizeof(char));
clientes[i].apellidos = (char *)malloc(200*sizeof(char));
clientes[i].direccion = (char *)malloc(200*sizeof(char));
clientes[i].viajes = (struct viaje *)malloc(0); // Init clientes[i].viajes to 0 as previously done with clientes
// Adding received element
strcpy(clientes[i].dni, dni);
strcpy(clientes[i].nombre, nombre);
strcpy(clientes[i].apellidos, apellidos);
strcpy(clientes[i].direccion, direccion);
/* DANGER - ERROR HERE */
clientes[i].totalViajes = 0; // HERE I GET A NOT EXPECTED BEHAVIOR - int is not added to clientes[i].totalViajes - Receive NULL
// New element added sucessfully
}
// SHOW ELEMENTS CREATED
for (i = 0; i < totalClientes; i++) {
printf("\n");
printf("%i.\n", i);
printf("NIF: %s\n", clientes[i].dni);
printf("NAME: %s\n", clientes[i].nombre);
printf("SURNAME: %s\n", clientes[i].apellidos);
printf("ADDRESS: %s\n", clientes[i].direccion);
printf("TRAVELS_COUNT: %s\n", clientes[i].totalViajes);
printf("\n");
}
// DELETING AN ELEMENT OF CLIENTES
int posCliente = 0;
printf("Input position of struct clientes that you wanna delete: ");
// posCliente is the position (inside the array) of cliente we wanna remove
scanf("%i", &posCliente);
fflush(stdin);
// Rewind one position array clientes since cliente we want to remove
for (i = posCliente; i < totalClientes-1; i++) {
// Rewind one element array clientes
clientes[i] = clientes[i+1];
}
//freeing memory of this element (now, the element is the last of the array, we have moved it into last position with previously for)
free(clientes[totalClientes].dni);
free(clientes[totalClientes].nombre);
free(clientes[totalClientes].apellidos);
free(clientes[totalClientes].direccion);
clientes[totalClientes].totalViajes = 0;
// Remove / free memory of the element deleted
/* DANGER - ERROR HERE */
//free(clientes[totalClientes]); // HERE I GET AN ERROR: `error: incompatible type for argument 1 of 'free'`
// Now totalClientes is one less (we have deleted one element)
totalClientes--;
// Resize array clientes. It must have one less element (now totalClientes is totalClientes-1)
/* DANGER - ERROR HERE */
//clientes = (struct cliente *)realloc(clientes, (totalClientes)*sizeof(struct cliente)); // HERE I DO NOT GET AN ERROR BUT PROGRAM CRASH
// SHOW ELEMENTS AFTER DELETING
/* DANGER - ERROR HERE */
// if the max legnth is totalClientes+1 we can see that the last element removed with free cointinues existing, so free is not freeing memory well
for (i = 0; i < totalClientes; i++) {
printf("\n");
printf("%i.\n", i);
printf("NIF: %s\n", clientes[i].dni);
printf("NAME: %s\n", clientes[i].nombre);
printf("SURNAME: %s\n", clientes[i].apellidos);
printf("ADDRESS: %s\n", clientes[i].direccion);
printf("TRAVELS_COUNT: %s\n", clientes[i].totalViajes);
printf("\n");
}
}
Several issues here. First:
printf("TRAVELS_COUNT: %s\n", clientes[i].totalViajes);
The field totalViajes
is an integer but you're using %s
to print, which expects a char *
pointing to a null terminated string. The value 0 is being interpreted as NULL pointer which is why NULL is being printed. Use %d
to print integers:
printf("TRAVELS_COUNT: %d\n", clientes[i].totalViajes);
Second, when you go to free
the strings for the deleted element, you're deleting the wrong one. You're using index totalClientes
, which is off the end of the array. Reading past the end of the array, as well as attempting to free
memory that wasn't received from malloc
, invokes undefined behavior.
You presumably wanted to do this for the last element, which has index totalClientes-1
, but that's not correct either, since that contains the strings for the last element in the list. The pointers to the memory you wanted to remove are in index posCliente
, which you overwrite when moving the element, causing a memory leak. Move the calls to free
to before the loop that shifts the elements, and use posCliente
as the index to clean up:
// Remove / free memory of the element deleted
free(clientes[posCliente].dni);
free(clientes[posCliente].nombre);
free(clientes[posCliente].apellidos);
free(clientes[posCliente].direccion);
// Rewind one position array clientes since cliente we want to remove
for (i = posCliente; i < totalClientes-1; i++) {
// Rewind one element array clientes
clientes[i] = clientes[i+1];
}
Finally, you can't call free
on the last element of the array, since 1) it's not a pointer, and 2) even if you took its address, that address wasn't returned by malloc
. The realloc
you have commented out is fine. The reason it crashed was due to undefined behavior that appeared earlier in your program.
Also, fflush(stdin)
is not valid, and gets
is insecure and should not be used because it could allow you to overrun your buffers. Use scanf
instead with length modifiers to specify the maximum size:
printf("Input NIF: ");
scanf("%254s", dni);
printf("Input NAME: ");
scanf("%254s", nombre);
printf("Input SURNAME: ");
scanf("%254s", apellidos);
printf("Input ADDRESS: ");
scanf("%254s", direccion);