confusion about free(pointer) in c

Question

I am trying to implement a stack using pointers, the structure definition is in the code below. I called the push() function for three elements (say: 2, 4, 6) to insert. Then, I call the function display(). It's showing only 0. I figured out the reason was because of the free() function in my push() function. But, i don't know what indeed is happening there. Should I not use free() to free the allocated memory used by temp in my code? If so, why?

#include<stdio.h>
//#include<unistd.h>
#include<stdlib.h> // malloc ,calloc, free avail here
void push();
void pop();
void display();
struct stack {
    int data;
    struct stack *next;
};

struct stack *start = NULL;

void push()
{
    int ele;
    struct stack *temp, *p;
    printf("entere the element\n");
    scanf("%d", &ele);
    temp = (struct stack *) malloc(sizeof(struct stack));
    temp->data = ele;
    if (start == NULL) {
        start = temp;
        start->next = NULL;
    } else {
        p = start;
        while (p->next != NULL) {
            p = p->next;
        }
        p->next = temp;
        temp->next = NULL;
    }
    free(temp);
}

@H2CO3 What is the reason not to cast the return value of malloc(). i guess we should type cast here.. isnt it? — Srinivas Thanneeru, Jul 23 '13 at 19:17
@SrinivasThanneeru It's useless, ugly and not easy to maintain. — nouney, Jul 23 '13 at 19:17
@SrinivasThanneeru I think you missed that `void *` is implicitly compatible with (convertible to and from) any object pointer type. It is in fact an error to cast it. [Explanation here.](http://stackoverflow.com/questions/605845/do-i-cast-the-result-of-malloc/605858#605858) — , Jul 23 '13 at 19:19
@H2CO3: `void*` is implicitly convertible, but not "compatible" with other pointer types; the Standard has definition of "compatible types" that's much stricter than implicit convertibility. Casting the result of `malloc` is generally a bad idea, but I wouldn't call it an "error"; I tend to reserve that word for things that require diagnostics and/or have undefined behavior. A cast can even be necessary if you want your code to compile as C++, though that's not a good idea nearly as often as some people think it is. — Keith Thompson, Jul 23 '13 at 19:27
@KeithThompson That's right. "Implicitly convertible" is the right expression to use. Again, this is the case of "illegal" which we argued about last time. For me, it **is** an error simply because it's terrible bad practice, and as a manager, I would immediately fire any C programmer who does that :) Also, as you mentioned too, compiling C code as C++ is an even worse idea. — , Jul 23 '13 at 19:34
@H2CO3: I suggest that when you use the word "illegal" without explaining just what you mean by it, a lot of people are going to assume that you mean it's an error that must be diagnosed. I also suggest that it's useful to distinguish between errors that must be diagnosed and bad style; if nothing else, it's important to understand that the latter will not necessarily be diagnosed by the compiler, and it's up to you as a programmer to avoid making such mistakes in the first place. By referring to style issues as "illegal", you make a good and valid point, but I don't think it's coming across. — Keith Thompson, Jul 23 '13 at 19:41

score 1 · Accepted Answer · answered Jul 23 '13 at 19:15

void push(){
               int ele;
               struct stack *temp,*p;
               printf("entere the element\n");
               scanf("%d",&ele);
               temp=(struct stack *)malloc(sizeof(struct stack ));
               temp->data=ele;
               if(start==NULL){
               start=temp;
               start->next=NULL;
            }
           else{
                p=start;
                while(p->next !=NULL){
                                       p=p->next;
                 }
                p->next=temp;
               temp->next=NULL;
            } 
            free(temp); // don't free temp here !  
          }

You have to free a pointer only if you don't need it any more. You could think it is the case, since you don't use temp, but it doesn't. The parameter of free is a valid memory address. temp is a valid memory address, but you're assigning temp to start ! So : free(tmp) is the same as free(start), and that's not what you want.

confusion about free(pointer) in c

1 Answers1