CODEWITHSUNDEEP
arrayscpointers
P. Gillich
P. Gillich

Reputation: 289

C program crashes after freeing the pointers in an array of char *

I'm a student learning C and I was puttering around with arrays of strings and malloc().

I have the following code that is supposed to load an array of strings (statically created) with dynamically created strings (please forgive / correct me if my terminology does not align with the code I have).

The problem is, once I go to free that memory, I get the following error: free(): invalid pointer

Here is the code:

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

#define RAM_SIZE 5

char* ram [RAM_SIZE];
int next_free_cell = 0;

void freeAndNullRam(){
    for (int i = 0 ; i < RAM_SIZE ; i++){
      printf("%d\n", i);
        free(ram[i]);
        ram[i] = NULL;
    }
}

int main(int argc, const char *argv[])
{
  for (int i= 0; i < RAM_SIZE; i++){
    ram[i] = (char*)malloc(sizeof(char*)*5);
    ram[i] = "aaaa";
  } 
  for (int i= 0; i < RAM_SIZE; i++){
    int empty = (ram[i] ==NULL);
    if(!empty){
      printf("%s\n", ram[i]);
    }
      
  }
  freeAndNullRam();
  for (int i= 0; i < RAM_SIZE; i++){
    int empty = (ram[i] ==NULL);
    printf("%d\n", empty);
  } 
  return 0;
}

I know the issue is definitely in the freeAndNullRam() function (obviously), but I don't understand why. My understanding is that at compile time, an array of 5 pointers to char arrays is created, but to actually fill the cells of the array, I need to malloc them some memory. Why does the program complain when I free the pointers in the array, but not when I give them memory?

Thanks!

Upvotes: 2

Views: 871

Answers (3)

Kuba hasn&#39;t forgotten Monica
Kuba hasn&#39;t forgotten Monica

Reputation: 98425

C's syntax strongly suggests that "aaaa" is a "string". People even talk of this syntax that way: they call it "strings". But "aaaa" is nothing such. It's the unfortunately named string literal, which is not a string - neither in C nor in C++. A char * is not a string either - it's a pointer-typed value. It's used to represent strings, but itself is not a string - not even close.

You have quite reasonably expected that "aaaa" might behave like any other rvalue of the "obvious" type. Alas, while 1 is an integer literal of type int, "aaaa" is a string literal of a pointer type const char * - its value is not a string, but a pointer!

It's as if when you wrote 42, C gave you a const int * pointing to 42. That's what "string" literals do. That's the awfully deplorable side of C :(

In C++, there actually is a string type (std::string), and you can even write literals of that type with a new syntax introduced in C++11: "aaaa"s is an rvalue* of type std::string, and you can assign them exactly as you would expect of any other value type like int.

Since you're already thinking a bit like in C++, perhaps you can investigate that language next. It takes much less effort to do plenty of basic things in C++ compared to C.

*technically rvalue reference

Upvotes: -1

tadman
tadman

Reputation: 211610

Here's a reworked version of your code to be more idiomatic C:

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

// Create an array of arbitrary size
char* alloc_array(size_t size) {
  // calloc() will give you a pre-zeroed (NULL) allocation, malloc() may not
  return calloc(size, sizeof(char*));
}

// Clears out all entries in the array, leaving only NULL
void clear_array(char* array, size_t size) {
  for (size_t i = 0; i < size; ++i) {
    // free(NULL) doesn't do anything, and is easier than a test
    free(array[i]);
    array[i] = NULL;
  }
}

// Clears, then frees the array
void free_array(char* array, size_t size) {
  clear_array(array, size);
  free(array);
}

int main(int argc, const char *argv[])
{
  // Whenever possible use local variables, not global variables
  size_t size = 5;
  char* entries = alloc_array(size);

  for (size_t i = 0; i < size; ++i) {
    // Make a copy with strdup() so this can be released with free()
    // later on. A string like "..." is static, it was never allocated.
    entries[i] = strdup("aaaa");
  } 

  for (size_t i = 0; i < size; i++) {
    // Express conditions in the if statment directly
    if (entries[i] != NULL) {
      printf("%s\n", ram[i]);
    }
  }
  
  clear_array(entries);

  for (size_t i = 0; i < size; i++) {
    printf("%d\n", entries[i] != NULL);
  }

  // Don't forget to release any allocated memory.
  free_array(entries);

  return 0;
}

There's a lot of bad habits in your original code you should work to expunge as quickly as possible so these things don't take root. In particular, global variables are a huge problem that need to be avoided.

One thing to remember is unless something was explicitly allocated with malloc() or a variant like calloc(), or was given to your code with an understanding that it was allocated in such a fashion, you should not call free() on it.

Not every pointer was allocated dynamically, and not every dynamically allocated pointer was allocated with malloc(). Some C code can be very confusing as a result of this.

Upvotes: 3

Oka
Oka

Reputation: 26355

ram[i] = "aaaa"; reassigns the pointers at a[i] to point to static memory, discarding the result of malloc. Later on you pass those pointers to free, which fails because they were not the result of an *alloc function.

Use strcpy to instead copy the string from static memory into your allocated destination.

strcpy(a[i], "aaaa")

Upvotes: 6

Related Questions