Error while trying to free memory

Question

I'm take some errors trying free memory. A post all my code below. I'm using ubuntu and I compile my code with gcc. But when I try execute my code I take an error while trying free memory. I put comments on my code to explain my doubt. I'm working with stack structre. How Do I free memory without take error to turn free memory for char? If I whoud not free the memory allocated for the data (a char), and only to free the memory for the element (which contains the char data), which happens with memory allocated to the data? is it free?

ERROR

{
    *** glibc detected *** ./pilha: free(): invalid next size (fast):      0x08b86018 ***
======= Backtrace: =========
/lib/i386-linux-gnu/libc.so.6(+0x74f82)[0xb7637f82]
./pilha[0x80485ba]
./pilha[0x804864c]
/lib/i386-linux-gnu/libc.so.6(__libc_start_main+0xf3)[0xb75dc4d3]
./pilha[0x8048411]
    ======= Memory map: ========
}

---

CODE

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

typedef struct Stack_element{
    char *data;
    struct Stack_element *next;

}Element;

typedef struct Position{
    Element *top;
    int size;
}stack;

void start(stack *aux){
    aux->top = NULL;
    aux->size = 0;
}

int push(stack *aux, char value){

    Element *new_element;
    if ((new_element = (Element*) malloc(sizeof(Element))) == NULL)
        return -1; //an error occur
    if ((new_element->data = (char*) malloc(sizeof(char))) == NULL)
        return -1; //an error occur

    strcpy(new_element->data, &value);

    new_element->next = aux->top;
    aux->top = new_element;
    aux->size++;

}

int empty(stack *aux){

    if ((aux->size) == 0){
        return -1;
    }
    return 0;

}

char pop(stack *aux){

    Element *element;
    char value='0';

    if (empty(aux)){
        return '1';
    }


    element = aux->top;
    aux->top = aux->top->next;

    /*
        To observe the line below. When a element exist in the stack and
        I try remove this element, first I free the data in that node (element)
        so I turn free memory allocated for the element.
        If I didn't free data memory allocated before (in push fuction), I
        don't get any error. But the memory allocated for the data, what happens?
        Does is it continues allocated?
    */

    value = *(element->data);
    free(element->data);//THE ERROR OCCURS HERE, IN THIS LINE
    free(element);//Just after free the data element memory, I also free the element's memory

    aux->size--;
    return value;
}

int main(){

    stack p;
    char value;

    start(&p);

    //no error occurs. there isn't any element at this moment.
    printf("%c\n",pop(&p));

    //valor = 't';
    if (push(&p, 't')){
        printf("Add a char\n");
    }
    pop(&p);//the error occurrs now, after insert an new element in the stack
    printf("The End.");

}

Answer 1

In your code,

strcpy(new_element->data, &value);

is not correct. You have allocated memory for only one char which is not having space for null terminator. Instead you should use

*(new_element->data) = value;

Otherwise, with the improper usage of strcpy(), you'e messing up the allocated memory by making memory overrung which causes undefined behaviour.

Answer 2

At least this statement in function push is invalid

strcpy(new_element->data, &value);

You should write

if ( ( new_element->data = (char*) malloc( 2 * sizeof(char))) == NULL)
{
    free( new_element );
    return -1; //an error occur
}

new_element->data[0] = value;
new_element->data[1] = '\0';

Or

if ( ( new_element->data = (char*) malloc( sizeof(char))) == NULL)
{
    free( new_element );
    return -1; //an error occur
}

*new_element->data = value;

Function empty looks strange. Usually -1 is returned from a function when some error occurred. As for the function empty then neither error can occur. So it is better when the function returns 1 if the stack is empty. I would write it like

int empty( stack *aux )
{
    return aux->size == 0; 
}

Also it is not clear why function pop returns character '1' when the stack is empty. It would be better if it returns simply '\0'. So instead of

char pop(stack *aux){

    Element *element;
    char value='0';

    if (empty(aux)){
        return '1';
    }
    //,,

I would write

char pop(stack *aux){

    if ( empty( aux ) ) return '\0';

    Element *element;
    //,,