CODEWITHSUNDEEP
cfilememory-managementstructbinary
Amarildo
Amarildo

Reputation: 268

Read from file and transfer on a dynamic struct

I'm trying to solve this exercise:

Create the files stringhe.h stringhe.c that let you use the following structures and function"

struct stringa {
    unsigned char length;
    char *s;
};

struct stringa *read_stringhe_bin(const char *filename, unsigned int *pn);

The structure contains the length field that contains the length of the string (possibly 0) and s field that points to a zero-terminated string (of length length). The function takes as a parameter a file name that is to be opened in read mode untranslated (binary) and a pointer to an unsigned int variable in which you will have to enter the number of strings in the file.

The file consists of a sequence of length elements variable in which a byte indicating the length n of the string and below there are n bytes containing the characters of the string.

The function must return a pointer to a new memory area (dynamically allocated heap) containing all strings read from the file. The number of strings is not known a priori and not limited.

It can be bound by the code. Even the s string element must be dynamically allocated heap.

This is my code:

stringhe.h

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

struct stringa {
    unsigned char length;
    char *s;
};

#ifndef READ
#define READ
struct stringa *read_stringhe_bin(const char *filename, unsigned int *pn);
#endif // !READ

stringhe.c

#include "stringhe.h"

struct stringa *read_stringhe_bin(const char *filename, unsigned int *pn) {
    FILE *f;
    f = fopen(filename, "r+b");
    if (f == NULL) exit(1);

    struct stringa *lista;
    lista = malloc(0);
    lista->s = malloc(0);

    for (int i = 0; ; i++) {
        char a;

        fseek(f, 1, SEEK_CUR);
        if (fgetc(f) == EOF) break;

        fseek(f, -2, SEEK_CUR);
        fread(&a, 1, 1, f);

        lista = (struct stringa *)realloc (lista, sizeof(struct stringa) * (size_t)(i + 1));
        lista[i].length = a;
        lista[i].s = malloc(sizeof(char) * (size_t)a);

        fread(lista[i].s, 1, (size_t)a, f);
        lista[i].s[a] = '\0';
        (*pn)++;
    }

    fclose(f);

    return lista;
}

I passed to the function the following binary files:

05 43 69 61 6F 21 00 03 61 62 63 0E 50 72 6F 67
72 61 6D 6D 61 7A 69 6F 6E 65

which is translated into:

.Ciao!..abc.Prog
rammazione

So... my function through debugging seems to work perfectly, however when I upload it to the website of our professor to see if it really works, it says that the main (created by the professor, and I can not see) ended incorrectly. So I doubt that I had done something wrong with the memory allocation or something similar. Could you give me a hand to understand the error and possibly improve this my code to make it more powerful?

Upvotes: 1

Views: 483

Answers (1)

chqrlie
chqrlie

Reputation: 144951

There are multiple problems with your code:

You open the file with "r+b" mode. Update is useless and error prone, just use "rb".

If the file cannot be opened, you might just return NULL instead of exiting the program.

lista = malloc(0);
lista->s = malloc(0);

malloc(0) may return NULL or the address of an object which you are not supposed to write to. You should instead do this:

lista = NULL;

When allocating the space for the string characters:

lista[i].s = malloc(sizeof(char) * (size_t)a);

You should allocate one extra byte for the terminating '\0' that you set after reading the bytes from the file. Do this:

lista[i].s = malloc(a + 1);

Seeking to check for end of file is cumbersome and may actually not be supported by the stream. Just read one byte for the size into an int, check for EOF and then allocate and read the specified number of bytes for the contents:

    int a;

    a = fgetc(f);
    if (a == EOF) break;

    lista = realloc(lista, sizeof(*lista) * (size_t)(i + 1));
    lista[i].length = a;
    lista[i].s = malloc(a + 1);
    lista[i].s[a] = '\0';

    if (fread(lista[i].s, 1, a, f) != a) {
        // file format error, ignore this incomplete string
        break;
    }

You should also check for malloc and realloc potential failure to allocate memory and return NULL gracefully.

Finally, the way you return the number of strings allocated is incorrect:

(*pn)++;

You do not know if *pn is 0 upon entry into your function... It might just be uninitialized in main(). Fix this by setting it once at the end of the parsing loop:

*pn = i;

Also why do you surround your function declaration in stringe.h with #ifndef READ etc. What is the purpose of this protection?

Upvotes: 2

Related Questions