Error when using malloc()

Question

I'm trying to import information from a file to a struct, but when I'm using malloc the program stop running. I've done similar functions before, I don't know why is not working.

This is my reading file function:

void ler_fich_salas(List_sala sala)
{
    FILE *fich;
    List_sala l;
    char linha[10];

    fich = fopen("fich_salas.txt","r");
    l = l->next;
    if (fich == NULL)
    {
        return;
    }
    else
    {
         /*ou l=l->next*/
        while (!feof(fich))
        {
            printf("A");
            fgets(linha, 10, fich);
            printf("Z");
            printf("%s",linha);/*testar se le bem no fich*/
            printf("B");
            free(l->nome_sala);
            l->nome_sala = (char *)malloc(TAM*sizeof(char));
            printf("C");
            strcpy(l->nome_sala, strtok(linha,"\n"));
            printf("D");

            l = l->next;

        }
    }
    fclose(fich);
}

and this is my struct:

typedef struct Sala_node *List_sala;
typedef struct Sala_node
{
    char *nome_sala;
    List_sala next;
}Cada_sala;

Any help would be appreciated! Thanks in advance.

You do nothing with the `sala` parameter. Is that intentional? — , May 28 '17 at 02:48
I would guess that the call to `free` on an unallocated value is your problem. — D.Shawley, May 28 '17 at 02:49
[Read about why you should not control file loops with `feof()`.](https://stackoverflow.com/questions/5431941/why-is-while-feof-file-always-wrong) — ad absurdum, May 28 '17 at 02:51
TAM = 1000. I should do with `sala` what I do with the `l`, but it was also doing an error. Also I called `free` because I thought that I was using too much memory, but I was just giving it a try. — Jessica Cunha, May 28 '17 at 02:58
this line: `l = l->next;` just after calling `fopen()`, is accessing some random place in memory because the `l` struct has not been initialized so `->next` is what every random value is on the stack at the location — user3629249, May 28 '17 at 03:08
in the code block beginning with: `if (fich == NULL)`, the code should output to `stderr`, the reason the system thinks the call to `fopen()` failed. Suggest: `perror( "fopen for reading fich_sales.txt failed" ); perhaps even followed by: `exit( EXIT_FAILURE );` — user3629249, May 28 '17 at 03:12
regarding this statement: `while (!feof(fich))`. The function: `feof()` does not do what you seem to think it does. Suggest: `while( fgets( linha, sizeof( linha ), fich ) {` — user3629249, May 28 '17 at 03:14
this line, *at least on the first pass through the loop* `free(l->nome_sala);` is passing to `free()` a pointer that has not been initialized to the returned value from (malloc or calloc or realloc). So that pointer in the struct contains what every value happens to be on the stack at the location. — user3629249, May 28 '17 at 03:17
when calling any of the heap allocation functions (malloc, calloc, reallc) 1) always check (!=NULL) the returned value to assure the operation was successful. 2) the returned value type is `void*` so can be assigned to an other pointer.. Casting the returned value just clutters the code, making it more difficult to understand, debug, etc. 3) the expression: `sizeof(char)` is defined in the C standard as 1. Multiplying anything by 1 has no effect. Suggest removing the casting, remove the expression: `sizeof(char)` and checking for an error return — user3629249, May 28 '17 at 03:21
when calling `strtok()`, always check (!=NULL) the returned value before using that value. — user3629249, May 28 '17 at 03:23
this line, after the call to `strcpy()`, l = l->next; is sourcing a field that has not been initialized, — user3629249, May 28 '17 at 03:25
Suggest searching stackoverflow.com for questions regarding linked lists. Then incorporate that information into your method of generating a linked list. — user3629249, May 28 '17 at 03:26
Note: if the code keeps changing the value in List_sala l;, then the beginning of the list pointer will be lost. Also, the resulting linked list is never passed back to the caller. If you plan on passing it back by the `sala` parameter, then that has to be declared as a `**` pointer, so the callers' pointer can be updated — user3629249, May 28 '17 at 03:30

score 1 · Answer 1 · answered May 28 '17 at 02:58

List_sala l;
/* SNIP */
l = l->next;

From the use of the -> operator, l must be a pointer type, which means you're hiding pointer types behind a typedef, which is a bad idea.

Even more problematic is that you haven't assigned it to point at anything in your logic, and so the pointer value points at gibberish. It's a miracle your program runs up until the call to free, let alone the statement after it that you claim is problematic, as that's trying to deallocate more gibberish (something that wasn't allocated using malloc) and then (yet again) dereference and assign to even more gibberish.

Either you've cut the wrong parts of your logic out to form an MCVE, rendering our ability to reproduce the issue impossible without filling in blanks (don't do that) or... my guess is that your book isn't working for you, as people who read decent books don't tend to have this kind of misunderstanding; believe me, I've been there! Please see this list to find a better book.

Thank's! I already realized what was my problem. In the main function I wasn't calling one function that create the list — Jessica Cunha, May 28 '17 at 03:08
No. Your *most significant* problem is that you gave us different code to what you used to reproduce the error. This kind of behaviour ruins the usefulness of *most* of your future questions. Please... In the future, make sure your MCVE *allows others to reproduce the issue*! — autistic, May 28 '17 at 03:23

Error when using malloc()

1 Answers1