0
votes

I'm learning C and I'm wondering why the code below didn't crash, I only picked up on the problem using Valgrind.

void push(char *element){
  FILE *file = fopen("stack.db", "w");
  int rc = fwrite(element, sizeof(element), 1, file);
  if (file) fclose(file);
}

void top() {
  char *top_element = malloc(sizeof(char));
  FILE *file = fopen("stack.db", "r+");
  int rc = fread(top_element, sizeof(char), 1, file);
  printf("Top element: %s", top_element);
}

int main(int argc, char *argv[]) {
  char action = argv[1][0];
  switch (action) {
    case 'p':
      printf("pushing element to stack\n");
      push(argv[2]);
      break;
    case 't':
      top();
      break;
    default:
      printf("die\n");
  }
  return 0;
}

First I call push() and write argv[2] to the file. Then I call top(); I get a piece of memory from malloc, the sizeof char, and assign it to top_element. However, this should be the sizeof char* so I am actually calling malloc(1) when I should have been calling malloc(8). This code works and I only picked up on the error when using Valgrind.

My question is, how does it work when the size of memory that I assigned to top_element was too small?

2
Writing beyond allocated memory is undefined behaviour. Crashing is just one of the possible outcomes.RedX

2 Answers

7
votes

No, it should be sizeof(char). You want a segment of memory long enough to store a single character; malloc(sizeof(char)) returns a pointer to exactly that. malloc(sizeof(char*)) would return a pointer to enough memory to store a char*. You can think of malloc(T * n) as returning a T* to n Ts.

I suspect Valgrind is complaining that printf is going to read beyond the end of the string represented by the char* top_element, as it's not zero terminated. If you want to read a string of n bytes from a file, you do need n + 1 bytes of memory, as you need to account for the \0 you should place on the end. However, not all char*'s represent strings, so this usage is fine in many cases. In your case, it really should be:

// Allocate zeroed memory
char *top_element = calloc(2, sizeof(char));

if (top_element)
{
  FILE *file = fopen("stack.db", "r+");
  int rc = fread(top_element, sizeof(char), 1, file);
  printf("Top element: %s", top_element);
  free(top_element);
}
else
{
  // Out of memory!
}
2
votes

You seem to be confused about char * and char:
A char can contain a character (duh!), while a char * is a pointer to a character. A pointer to a character can also be a string, but it doesn't have to be. A string is a number of consecutive chars, terminated by a null-byte.

char *top_element = malloc(sizeof(char));

Asks malloc() for memory to store a single char, not a string.

printf("Top element: %s", top_element);

Tells printf() to expect a string.
Since top_element really is just a char, it needs not be null-terminated. If it isn't, printf() will read past the allocation, which is undefined behaviour (if you're lucky, printf() will hit a page boundary to an unallocated page, causing a page-fault). The reason that you've not crashed is that uninitialised memory will often contain zero-bytes, which will act like the null-terminator for printf(). This is not guaranteed however, and relying on it would be very dangerous.