Search code examples
cfreadstdiounsigned-char

C function to read binary file doesn't work if used by other function.?


I wrote this function to return an unsigned char * containing the data from any file(using rbread mode) it looks like this:

uchar * get_file_data (char *f_name, luint *len) {

    // verify file
    struct stat sBuffer;
    int iRc;

    iRc = stat(f_name, &sBuffer);
    if (iRc != 0) {
        printf("Failed to get file size\n");
        exit(3);
    }

    // read file
    FILE *fp;
    fp = fopen(f_name, "rb");
    if (fp == NULL) {
        printf("Failed to open file\n");
        exit(3);
    }

    uchar *buff = (uchar *) malloc(sBuffer.st_size);
    *len = fread(buff, 1, sBuffer.st_size, fp);
    fclose(fp);

    return buff;

}

Types:

typedef unsigned char uchar;
typedef unsigned long int luint;

Now this works when run from the int main() function like so:

luint *len;
uchar *dat = get_file_data(argv[2] /* argument congaing filename */, len);
print_uchar(dat, len);
free(dat);

However when calling has_checksum(argv[2]); from int main() i get a Segmentation fault (core dumped) though the use is seemingly the same.

int has_checksum (char *f_name) {

    luint *p_len;
    uchar *p_f_data = get_file_data(f_name, p_len); /* this fails */

    /* more code, never reached */

}

When i try running my program with the following main():

int main(int argc, char **argv) {

    // verifying other arguments (not filename) */

    luint *len;
    uchar *dat = get_file_data(argv[2], len);
    print_uchar(dat, len);
    free(dat);

    has_checksum(argv[2]);

    return 0;
}

I get this output:

$ ./md5 -add ../<filename> 
<file contents (correct)>
Segmentation fault (core dumped)

I tested if char *f_name in get_file_data() was the same each time(using printf("%s\n", f_name); immediately when entering get_file_data()), and it was.


System:
-- VM Ubuntu 17.04 (64-bit) -- VirtualBox


Solution

  • You're passing an uninitialized pointer in both cases, but you're getting "lucky" in main() and it appears to work. You need to pass the address of an actual integer:

    luint len;
    uchar *dat = get_file_data(argv[2], &len);
    

    You see, when you declare luint *len;, you've declared a pointer but nothing for it to point at. It contains an uninitialized value and may point anywhere. You may or may not be able to write to that location.