Why is this C function crashing at fclose?

Question

Please help me understand why this function throws an exception when it reaches fclose :

void receive_file(int socket, char *save_to, int file_size) {
    FILE *handle = fopen(save_to,"wb");
    if(handle != NULL) {
        int SIZE = 1024;
        char buffer[SIZE];
        memset(buffer,0,SIZE);
        int read_so_far = 0;
        int read_now = 0;
        int reading_unit = (file_size < SIZE) ? file_size : SIZE;
        do {
            read_now = read(socket,buffer,reading_unit);
            fwrite(buffer,read_now,1,handle);
            read_so_far += read_now;
            if(read_so_far >= file_size) {
                break;
            }
            memset(buffer, 0, sizeof(buffer));
        } while (1);
        read_now = 0;
        fclose(handle);
    }
    else {
        extern int errno;
        printf("error creating file");
        printf(" error code : %d",errno);
        exit(-1);
    }
}

Eclipse CDT exists with the following error :

Single stepping until exit from function __kernel_vsyscall, 
which has no line number information.

The purpose of this function is to receive a file through a socket.

EDIT: I'm using CentOS 5.3. The thing is, the file is created and written. Even the MD5 is correct. I don't understand why it's failing at fclose.

EDIT2: here is a stack trace I managed to get :

*** glibc detected *** /home/linuser/workspace/proj/Debug/proj: free(): invalid next size (normal): 0x096a0068 ***
======= Backtrace: =========
/lib/libc.so.6[0x4fb0f1]
/lib/libc.so.6(cfree+0x90)[0x4febc0]
/lib/libc.so.6(fclose+0x136)[0x4e9c56]
/home/linuser/workspace/proj/Debug/proj[0x8048cd8]
/home/linuser/workspace/proj/Debug/proj[0x80492d6]
/home/linuser/workspace/proj/Debug/proj[0x804963d]
/lib/libc.so.6(__libc_start_main+0xdc)[0x4a7e8c]
/home/linuser/workspace/proj/Debug/proj[0x8048901]
======= Memory map: ========
001ff000-00200000 r-xp 001ff000 00:00 0          [vdso]
0046f000-00489000 r-xp 00000000 08:06 1280361    /lib/ld-2.5.so
00489000-0048a000 r-xp 00019000 08:06 1280361    /lib/ld-2.5.so
0048a000-0048b000 rwxp 0001a000 08:06 1280361    /lib/ld-2.5.so
00492000-005d0000 r-xp 00000000 08:06 1280362    /lib/libc-2.5.so
005d0000-005d2000 r-xp 0013e000 08:06 1280362    /lib/libc-2.5.so
005d2000-005d3000 rwxp 00140000 08:06 1280362    /lib/libc-2.5.so
005d3000-005d6000 rwxp 005d3000 00:00 0 
005d8000-005fd000 r-xp 00000000 08:06 1280369    /lib/libm-2.5.so
005fd000-005fe000 r-xp 00024000 08:06 1280369    /lib/libm-2.5.so
005fe000-005ff000 rwxp 00025000 08:06 1280369    /lib/libm-2.5.so
009b2000-009bd000 r-xp 00000000 08:06 1280372    /lib/libgcc_s-4.1.2-20080825.so.1
009bd000-009be000 rwxp 0000a000 08:06 1280372    /lib/libgcc_s-4.1.2-20080825.so.1
009c5000-00aa5000 r-xp 00000000 08:06 5465873    /usr/lib/libstdc++.so.6.0.8
00aa5000-00aa9000 r-xp 000df000 08:06 5465873    /usr/lib/libstdc++.so.6.0.8
00aa9000-00aaa000 rwxp 000e3000 08:06 5465873    /usr/lib/libstdc++.so.6.0.8
00aaa000-00ab0000 rwxp 00aaa000 00:00 0 
08048000-0804a000 r-xp 00000000 08:06 4884214    /home/linuser/workspace/proj/Debug/proj
0804a000-0804b000 rw-p 00001000 08:06 4884214    /home/linuser/workspace/proj/Debug/proj
096a0000-096c1000 rw-p 096a0000 00:00 0          [heap]
b7e00000-b7e21000 rw-p b7e00000 00:00 0 
b7e21000-b7f00000 ---p b7e21000 00:00 0 
b7f99000-b7f9a000 rw-p b7f99000 00:00 0 
b7faa000-b7fac000 rw-p b7faa000 00:00 0 
bfa82000-bfa97000 rw-p bffea000 00:00 0          [stack]

and here is the "updated" version of the function :

void rec_file(int socket,char *path,int size)
{
    FILE *handle = fopen(path,"wb");
    char buffer[4096];
    int total_read = 0;
    if(handle != NULL)
    {
        while(1)
        {
            int bytes_read = recv(socket,buffer,4096,0);
            total_read    += bytes_read;
            if(bytes_read != -1)
            {
                fwrite(buffer,bytes_read,1,handle);
            }
            else
            {
                printf("read error ");
                exit(-1);
            }
            if(total_read >= size)
            {
                break;
            }
        }
        fclose(handle);
    }
    else
    {
        printf("error receiving file");
        exit(-1);
    }
}

maybe this is cleaner? However, I still receive the same fclose exception.

EDIT3: I commented out everything and I only left the following code, which sadly, still throws the exception at fclose :

void nrec(int sock,char* path,int size)
{
    FILE *handle = fopen(path,"wb");
    if(handle == NULL)
    {
        printf ("error opening file");
        return;
    }
    fclose(handle);
}

Solution 1

• fclose() returns a value: 0 if the file is closed successfully, EOF if not. Consider modifying your code to ...

  if (fclose(handle)) { printf("error closing file."); exit(-1); }

• As everyone else points out, check read() for error.

• If the crash is happening after the fclose returns 0 (success), and all the read()s were successful, then perhaps the trouble is elsewhere. Maybe it's the VLA. Consider changing the buffer code to a file static or a malloc / free.

• If the caller was mistaken about the file size, say, and claimed the file was 500 bytes when in fact it is only 480, will your code keep reading the socket forever?

Solution 2

Not sure specifically, but you do know that read() can return a negative on error, right?

Solution 3

This code seems very ... uncertain about itself. Lot of needless assignments, memset() calls, complicated logic.

There's no point in limiting the amount you try to read. Always read as much as you have buffer space for; if there's less data available, you will get less.

It also doesn't properly handle errors. If read() fails, it can return -1, in which case Bad Things(TM) will happen all over the place.

My advice would be to clean it up, think about return values from I/O functions and check them better, then try again.

Failing that, if you're in Linux, run it through Valgrind to get help tracking down the crash.

Solution 4

You didn't check whether you got an error indication from the read() call. If you get a -1 from read(), you then pass it, unchecked, to fwrite(), which takes a size_t as its argument. When your int value is converted to size_t (or treated as size_t), it becomes a rather large number (4 GB - 1 on a 32-bit system). So, fwrite() does as you command - tries to write a lot of data from places it is not supposed to.

Solution 5

The code in edit3 looks reasonable. I suspect you've probably stomped over some memory somewhere else and are reaping the problem at this point in your application.

Do you have bounds checkers, purify etc that you could run to check for buffer overruns, using a free'd memory block etc.

View more solutions

Comments

[Admin]

Either the compiler should accept it (if it is in C99 mode) or reject it as a syntax error.

[Thomas L Holaday]

The compiler might accept it and then implement it with "crash on exit." New features, new learning opportunities.

[MarkD]

It blows up when it enters fclose. fclose doesn't get the chance to finish and return.

[Thomas L Holaday]

If you comment out everything but the fopen and fclose, does it still crash? If you change the reading unit to (file_size : SIZE-10) so that you have some good-luck bytes at then end, does it still crash?

[MarkD]

I commented everything but fopen and fclose and it still crashes. Please see the new edit.

[MarkD]

I want to thank you for your extremely helpful comments. I managed to detect the error, it wasn't in this function. I was malloc'ing a buffer inside a function "incorrectly" and it all backfired in this one. Thank you very much !