0

can you tell me why this program sometimes returns 0xc0000005 (crash) and sometimes it runs well? I am beginner in C and have no idea what is wrong. I am using GNU GCC compiler and Code::Blocks. Thank you in advance.

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

typedef struct member {
int number;
char *line;
struct member *next;
}MEMBER;

typedef MEMBER *LINKED_LIST;

int main() {
char file_line [120], *str;
FILE *fi, *fo;
LINKED_LIST curr, head;
int count = 0, count_sort = 0 , end_sort, tmpint;

fi = fopen("vstup.txt","r");
if(fi == NULL){
    printf("vstup.txt - ERROR when opening file/file not found\n");
    return 0;
}
curr = (LINKED_LIST)malloc(sizeof(LINKED_LIST));
head = curr;
while(fgets(file_line,sizeof(file_line),fi))
{
    curr->number = strtol(file_line, &str,10);
    while(isspace(*str))
        str++;
    curr->line = strdup(str);
    curr->next = (LINKED_LIST)malloc(sizeof(LINKED_LIST));
    curr = curr->next;
    memset(&file_line,0,120);
    count++;
}
curr->next = NULL;
curr = NULL;
curr = head;
count_sort = count-1;
end_sort = count;
while(end_sort)
{
    if((curr->number) > (curr->next->number))
    {
        tmpint = curr->number;
        curr->number = curr->next->number;
        curr->next->number = tmpint;

        str = curr->line;
        curr->line = curr->next->line;
        curr->next->line = str;
    }
    curr = curr->next;
    count_sort--;
    if(count_sort == 0)
    {
        count_sort = count-1;
        curr = head;
        end_sort--;
    }
}
curr = head;
fclose(fi);
if (count == 0)
{
    printf("List/file is empty.\n\n");
    return 0;
}
fo = fopen("vystup.txt","w");
while(count)
{
    fprintf(fo,"%s", curr->line);
    curr = curr->next;
    count--;
}
printf("Success!\n");
fclose(fo);
return 0;
}
Peter
  • 449
  • 1
  • 3
  • 13
  • 5
    debug it?............. – Mitch Wheat Dec 31 '14 at 00:27
  • 1
    You probably have [*undefined behvior*](http://en.wikipedia.org/wiki/Undefined_behavior) somewhere. Rebuild with more warnings enabled, and fix their root cause and try again. If that doesn't work, run in a debugger to pinpoint the location. – Some programmer dude Dec 31 '14 at 00:33
  • You also have a memory leak. When you have read the last line, `curr->next` point to allocated memory, and then you reassign `curr->next` to `NULL making you loose that memory. – Some programmer dude Dec 31 '14 at 00:39
  • 1
    @datenwolf, yes it does. `file_line` is an array, its address is the same as its pointer-coerced value. the & isn't needed, but isn't harmful here. – Jasen Dec 31 '14 at 01:47

2 Answers2

5

You've created a typedef for a pointer

typedef MEMBER *LINKED_LIST;

Then you've declared a couple of pointers using that typedef

LINKED_LIST curr, head;

When you allocate space for curr you probably intended to allocate space for a copy of the structure, but instead you allocated space for a pointer.

curr = (LINKED_LIST)malloc(sizeof(LINKED_LIST));

So the following line results in undefined behavior because you haven't actually allocated space for the structure.

    curr->number = strtol(file_line, &str,10);

Moral of the story: don't create typedef's for pointers.
Side note: also don't cast the return value of malloc.

user3386109
  • 34,287
  • 7
  • 49
  • 68
  • For why `don't cast the return value of malloc`, check [this](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc) – D3Hunter Dec 31 '14 at 00:46
  • `Moral of the story: don't create typedef's for pointers.`.. I humbly disagree. Rather, its better to use a particular nomenclature type to avoid confusion, like `pLINKED_LIST`. Personal opinion, though. – Sourav Ghosh Dec 31 '14 at 05:55
  • 1
    @Sourav Ghosh: create typedefs for structures, then let the '*' asterix be the obvious pointer bit of any subsequent usage. – Mitch Wheat Dec 31 '14 at 06:35
2

This line is very suspicious:

if((curr->number) > (curr->next->number))

What if curr is the last node in the list? Then you dereference a NULL pointer.

Some programmer dude
  • 400,186
  • 35
  • 402
  • 621