corrupted size vs. prev_size Abortado (`core' generado) with function fclose

Question

i am trying to do script files to create and extract tarballs. It is code fro college and I am very new programming with c. Anyway, I can create a tarball, btw the problem comes when i wanted to extract, due to fclose function. Everything seems right and I can't find the issue. All the program runs ok, but when I tried to close the tarFile with fclose(tarFile) it gives me thisa message:

Output:

corrupted size vs. prev_size
Abortado (`core' generado)

By debugging I detected where is the problem. It occurs when the header[i].size is assigned inside the readHeader function, in the for loop.

This error is the only thing i need to fix to finish the exercise, because the data from the tarball is copied correctly, but it is important to close de tarFile so i appreciate any type of help.

Thank you so much and sorry if i've been able to explain it better. This is my first post in stackoverflow and english is no my native language.

Here is the code

• Extract function

/** Extract files stored in a tarball archive
 *
 * tarName: tarball's pathname
 *
 * On success, it returns EXIT_SUCCESS; upon error it returns EXIT_FAILURE. 
 * (macros defined in stdlib.h).
 *
 * HINTS: First load the tarball's header into memory.
 * After reading the header, the file position indicator will be located at the 
 * tarball's data section. By using information from the 
 * header --number of files and (file name, file size) pairs--, extract files 
 * stored in the data section of the tarball.
 *
 */
int
extractTar(char tarName[])
{
    //1. Load Headers
    FILE* tar = NULL;
    tar = fopen(tarName, "r");
    if(tar == NULL){
        printf ("Error al abrir el tarball\n");
        return EXIT_FAILURE;
    }
    printf("ADDRES INIT: %p\n", &tar);
    int nFiles = 0;
    stHeaderEntry* header = NULL;
    //Carga el header y nFiles
    header = readHeader(tar, &nFiles);
    if(header == NULL || nFiles <= 0){
        printf("Error al crear header // NUM FILES: %d\n", nFiles);
        return EXIT_FAILURE;
    }
    printf("Copiando...\n");
    
    //3. Copia de datos
    //Para este punto el tar debería apuntar a la sección de datos
    //por lo que se podría pasar a copiar los datos en función
    //del header anteriormente cargado
    FILE* output = NULL;
    for(int i = 0; i < nFiles; i++){
        output = fopen(header[i].name, "w");
        if(output == NULL){
            printf("Error al abrir output\n");
            return EXIT_FAILURE;
        }
        int n = copynFile(tar, output, header[i].size);
        if(n == -1){
            printf("Error al copìar los datos en output\n");
            return EXIT_FAILURE;
        }

        if(fclose(output) != 0){
            printf("Error al cerra output\n");
            return EXIT_FAILURE;
        }
    }   


    printf("LLAMADA A FREE\n");
//-------------FREE_MEMORY-----------------------//
    printf("%s\n", header[0].name);
    for(int i = 0; i < nFiles; i++){
        printf("FREE HEADER NAME\n");
        free(header[i].name);
        header[i].name = NULL;
    }   
    
    printf("FREE HEADER\n");
    free(header);
    
    printf("CLOSE TAR\n");
    printf("ADDRES END: %p\n", &tar);
    if(fclose(tar) != 0){
        printf("Fallo al cerrar tarFile\n");
        return EXIT_FAILURE;
    }
    
    printf("Extracción realizada con éxito\n");
    return EXIT_SUCCESS;
}

• readHeader Function

/** Read tarball header and store it in memory.
 *
 * tarFile: pointer to the tarball's FILE descriptor 
 * nFiles: output parameter. Used to return the number 
 * of files stored in the tarball archive (first 4 bytes of the header).
 *
 * On success it returns the starting memory address of an array that stores 
 * the (name,size) pairs read from the tar file. Upon failure, the function returns NULL.
 */
stHeaderEntry*
readHeader(FILE * tarFile, int *nFiles)
{
    int nrFiles = 0;
    //Lectura de nFiles
    if((fread(&nrFiles, sizeof(int), 1, tarFile)) != 1){
        printf("Error al leer nFiles\n");
        return NULL;
    }
    
    //Reserva de memoria en función de nFiles
    stHeaderEntry* header = malloc(sizeof(stHeaderEntry) * (*nFiles));

    //Lectura de los datos del header
    char* str = NULL;
    
    int size = 0;
    for(int i = 0; i < nrFiles; i++){
        //Primero el nombre
        str = loadstr(tarFile);
        if(str == NULL){
            return NULL;
        }
        
        header[i].name = malloc(sizeof(str) + 1);
        header[i].name = strcpy(header[i].name, str);
        header[i].name = strcat(header[i].name, "\0");
    
        
        //Segundo los bytes del archivo
        int n = fread(&size, sizeof(unsigned int), 1, tarFile);
        if(n != 1){
            printf("Error al leer header size\n");
            return NULL;
        }

        header[i].size = size;
    }       

    //Carga completa
    printf("READHEADER SUCCESSFUL\n");
    (*nFiles) = nrFiles;
    return header;
}

Answer 1

In your readHeader function you allocate space for a string as follows:

header[i].name = malloc(sizeof(str) + 1);

This will use the size of str, a string pointer which will be a constant value depending on the size of a pointer of you system (usually 4 or 8); this is not what you want.

You should be using strlen to calculate the length of the C string pointed to by str.

header[i].name = malloc(strlen(str) + 1);

There is a problem in loadstr as well, since you only read one character into the buffer, you should change:

    while((n = fread(buf, 1, 1, file)) != 0 && strcmp(buf, "\0") != 0){
        length++;
    }

to

    while((n = fread(buf, 1, 1, file)) != 0 && buf[0] != '\0'){
        length++;
    }

otherwise strcmp might be reading from uninitialized memory.

A few other observations:

• You don't really need to copy the string since loadstr is allocating it, but if you do then you also need to free the return from loadstr.
• You don't need to null terminate the string after strcpy it will do it for you.
• Read up on strncpy it's a safer option than strcpy.

Answer 2

• loadstr contains this

/** Loads a string from a file.
 *
 * file: pointer to the FILE descriptor 
 * 
 * The loadstr() function must allocate memory from the heap to store 
 * the contents of the string read from the FILE. 
 * Once the string has been properly built in memory, the function returns
 * the starting address of the string (pointer returned by malloc()) 
 * 
 * Returns: !=NULL if success, NULL if error
 */
char*
loadstr(FILE * file)
{
    char* str = NULL;
    char buf[1];
    int length = 0;
    int n = 0;
    //Asignación del tamaño del buffer
    while((n = fread(buf, 1, 1, file)) != 0 && strcmp(buf, "\0") != 0){
        length++;
    }
    
    str = malloc(length + 1);
    
    //Debido al while anterior, ahora file apunta a otro lado
    //Hay que hacer un rewind
    fseek(file, -(length + 1), SEEK_CUR);
    
    //Se guarda el nombre en str
    if(fread(str, length + 1, 1, file) != 1){
        printf("Error en loadstr\n");
        return NULL;
    }
    return  str;
}

I didn't post before because i thought it was unnecessary.