char * str = (char*) malloc( ) Crash help!

Question

Hello guys,

I have this code snippet below and it crashes during the assignment in 'str', a dynamic allocation.

  char *str;
  int file_size;
  FILE *fptr;
  if (!(fptr = fopen(filename, "r"))) goto error1;
  if ((fseek(fptr, 0L, SEEK_END) != 0)) goto error2;
  if (!(file_size=ftell(fptr))) goto error2;
  if ((fseek(fptr, 0L, SEEK_SET) != 0)) goto error2;
  str =  (char*)malloc(file_size+1);
  if (fread(str, file_size, 1, fptr) != 1) {
    free(str);
    goto error2;
  }
  str[file_size] = '\0';
  fclose(fptr);

Can somebody help me? I am using ARM.

edit : file_size is non-zero, non-negative less-140 value This actually works on my intel pc, but not on arm machine.

Thanks, giantK

Answer 1

It might be a good idea to RTFM - ftell() returns -1 on error, not zero.

Answer 2

Print your variables out, particularly file_size, before you use them. You may get a surprise.

Answer 3

What is the value, returned by ftell(fptr)? Maybe it is too large? If it returns long int indeed, then it might overflow your int and you'll get a negative value there.

Answer 4

On a general sense... I'm not meaning to bash at all, but your code is a bit awful. Using coding standards will usually make things a lot clearer for you... That said, Neil probably got the answer.

Answer 5

malloc expects a size_t for argument. size_t is a typedef for unsigned int or unsigned long (depending on platform), the key here being UNSIGNED.

You're using an int for file_size, and an int could be JUST 16bits (you're using an ARM, so I'm gonna think this is an MCU). A signed 16 bits can only support 32,768 bytes of file size in bytes, so, if you have a big file (not that big actually, just >32K), file_size will overflow.

I think the compiler told you that, but you choose to ignore it... now mallow takes an unsigned argument, so it automatically casts your signed evaluation of filesize+1 (even if it's deeply overflown, a signed big time negative) and tries to allocate memory. This could mean that you tried to allocate far too much memory than this embedded app. can't have (shouldn't have crashed).

I don't really see a reason for crashing (other than a bad library, which in embedded C is common, due to low users base feedback), but I see errors that lead to unwanted behavior.

I'm not even go and ask "why the goto stuff", 'cause the answer would trigger a lot of flame comments.

Answer 6

someplace before you do buffer overflow OR free() on improper address!!!-)

Answer 7

It is unlikely that this is your problem, but did you remember to

#include <stdlib.h>

so that the prototype of malloc is in scope? The compiler would not warn you in this case because of the gratuitous cast of its return value. In C, there is no reason to cast the return value of malloc. So:

str =  malloc(file_size + 1);

Incidentally, whitespace is free.

Finally, you should open the file in binary mode if you want to be able to deduce its size this way on multiple platforms.

Based on:

str[file_size] = '\0';

you seem to be implicitly assuming that the file cannot contain embedded \0 characters. That strikes me as a dangerous assumption if you indeed are making it.