free is leading to memory corruption [closed]

Question

This question is unlikely to help any future visitors; it is only relevant to a small geographic area, a specific moment in time, or an extraordinarily narrow situation that is not generally applicable to the worldwide audience of the internet. For help making this question more broadly applicable, visit the help center.

Closed 8 years ago.

Friends, I am trying to free memory from pointer array as:

const gchar *strings[21];
strings[0]  = malloc(strAuth)
strings[0]  = strAuth
....
....
    int j=0;
    while(j < 20){
      if (strlen(strings[j]) != 0) {
    g_free((char*)strings[j]);
    g_print("Cleaned:%d\n",j);
      }
      j++;
      g_print("%d",j);
    }
//g_free((char*)strings);

j prints upto 20 and then gives

$ ./mkbib 
Cleaned:0
1Cleaned:1
2Cleaned:2
34Cleaned:4
56789101112Cleaned:12
1314151617181920*** glibc detected *** ./mkbib: malloc(): memory corruption (fast): 0x0000000000a14e10 ***

Any explanation (to a C-novice)?

EDIT 1 Sorry for the stupid info, I was avoiding what strAuth is because that involves gtk library(I have bad experience about asking specific library dependent question in clc). So the real code looks:

 strings[0]  = g_malloc(strlen(gtk_entry_get_text(GTK_ENTRY(e->entry1))));
 strings[0]  = gtk_entry_get_text(GTK_ENTRY(e->entry1));

where gtk_entry_get_text is of type const gchar * Possibly I have wasted you time with initial post. Please help.

EDIT 2

const gchar *strings[21]; 
strings[0] = g_malloc(strlen(gtk_entry_get_text(GTK_ENTRY(e->entry1)))); 
strings[0] =g_strdup(gtk_entry_get_text(GTK_ENTRY(e->entry1))); 
........
int i=2; 
    while (i < 21) {
      if (strlen(strings[i]) != 0) {
    g_string_append_printf(tstring, ",\n\t%s=\"%s\"",
        keyword[i], strings[i]);
      g_free((char*)strings[i]);
      strings[i]=NULL;
      g_print("Cleaned:%d\n",i);
      } 
      i++;
 }

Answer 1

Firstly, this

strings[0]  = malloc(strAuth)
strings[0]  = strAuth;

is defintely broken. What is the type of strAuth? How did you manage to use strAuth as argument of malloc (i.e. size) and then immediately as the right-hand size of assignment? mallocs argument must be an integer, while strings[0] has pointer type. Aside from being completely self-contradictory, this sort of usage will trigger diagnostic messages from the compiler. Did you just ignore those messages?

If strAuth is a string and if you are trying to allocate memory for a copy of strAuth, then the typical memory allocation idiom would be

strings[0] = malloc(strlen(strAuth) + 1);

Secondly, why are you even attempting to assign anything to strings[0] after malloc? The pointer in strings[0] is your only connection to the freshly allocated memory, which you should cherish and preserve by all means. Instead you immediately clobber that pointer by assigning a new value to strings[0], turning the memory you just allocated into a memory leak.

Again, if you are trying to create a copy of strAuth in strings[0], then the typical idiom would be

strings[0] = malloc(strlen(strAuth) + 1);
strcpy(strings[0], strAuth);

(of course, in real code one should always remember to check whether malloc succeeded or not).

On many platforms a non-standard strdup function is available, which wraps exactly the above allocate-and-copy functionality, meaning that the above two lines can be replaced with simple

strings[0] = strdup(strAuth);

And finally, thirdly, what is g_free? Are you sure it is applicable to memory allocated by standard malloc (as opposed to, say, g_malloc). And even if it happens to be applicable, it is still not a good idea to mix APIs like that. If you want to allocate memory by standard malloc (or strdup), then it is a good idea to stick to standard free for freeing it.

Answer 2

strings[0]  = malloc(strAuth)
strings[0]  = strAuth

After allocating memory for strings[0], you overwrite the returned pointer with strAuth, which I don't know what it is, but probably a string which was not allocated using malloc() (or one of its relatives like realloc()). And you can't free such an object.

(and anyways: if strAuth is a string, shouldn't you allocate enough space for its length (plus one for the terminating NUL byte) instead? malloc(strAuth) seems nonsensical to me.)

Answer 3

Let's add some comments to help you out:

const gchar *strings[21];     // allocate an array of 21 pointers to gchar constants
strings[0]  = malloc(strAuth) // assign some memory (strAuth number of bytes) to the
                              //   first pointer
strings[0]  = strAuth         // reassign the first pointer the value of strAuth

When you do the assignment of strAuth to strings[0] you're overwriting what was malloc'd there in the first place. So if strAuth is some string literal or something not malloc'd then a call to free would indeed fail. You can only free what has been dynamically allocated.

So you need to decide if you want to stick pointers to constant strings in your array (and don’t malloc or free) or if you want to malloc/free and then copy your strings into the array.

Note you're calling malloc() in conjunction with g_free() that's not a good idea. g_malloc() and g_free() are more than just wrappers around free() and malloc() and so you should use them as such... pseudo-code explanation:

if 
    g_malloc(strings[x]) 
  then 
    g_free(strings[x])
else if 
    strings[x] = malloc(y) 
  then 
    free(strings[x])