Solved

Easy Function question

Posted on 2004-09-22
8
204 Views
Last Modified: 2010-04-15
I have created a self referential structure and am creating a stack to do tree searches, when I push onto the stack everything is fine, but when I pop the first element off the stack and free the memory for the popped item the previous node is also "freed". So the first iteration of my for loop to print out the stack recieves 'f', but then doesn't seem to be pointing to the previous node (or the previous node has been corrupted... Here is the code:


struct DL_Node{
      char element;
      struct DL_Node *prev;
      struct DL_Node *next;
};

struct DL_Node *push(struct DL_Node *n, int a){
      struct DL_Node *p;

      if (!n->element)
            n->element = a;
      else{
            p = malloc(sizeof(struct DL_Node));
            p->element = a;
            p->prev = n;
            p->next = NULL;
            n->next = p;
            n = p;
      }
      return n;
}

char pop(struct DL_Node *n){
      char a;

      a = n->element;
      if (n->prev){
                      n = n->prev;
            free(n->next);
            n->next = NULL;
      }
      else
            free(n);

      return a;

}

void main(){
      struct DL_Node *s;
      struct DL_Node *root;
      int i;
      root = malloc(sizeof(struct DL_Node));
      root->element = (char) NULL;
      root->next = NULL;
      root->prev = NULL;

      s = push(root, 'c');
      s = push(s, 'd');
      s = push(s, 'e');
      s = push(s, 'f');

      while (s->prev)
            printf("%c\n", pop(s));

}
0
Comment
Question by:raistlinx17
  • 4
  • 3
8 Comments
 
LVL 5

Expert Comment

by:tzxie2000
ID: 12122700
the problem is because
when you call
char pop(struct DL_Node *n)

you assigned  n = n->prev; and expect n to point to the next value

but the value n is called by point value so it is not change
0
 
LVL 5

Expert Comment

by:tzxie2000
ID: 12122730
you could change code as

struct DL_Node{
     char element;
     struct DL_Node *prev;
     struct DL_Node *next;
};

struct DL_Node *push(struct DL_Node *n, int a){
     struct DL_Node *p;

     if (!n->element)
          n->element = a;
     else{
          p = malloc(sizeof(struct DL_Node));
          p->element = a;
          p->prev = n;
          p->next = NULL;
          n->next = p;
          n = p;
     }
     return n;
}

char pop(struct DL_Node **p){//<-------------change to the reference
     char a;

       struct DL_Node *n;
     n=*p;
 
     a = n->element;
     if (n->prev){
                     n = n->prev;
          free(n->next);
          n->next = NULL;
     }
     else
     {
          free(n);
         n=NULL;
    }
    *p=n;
     return a;

}

void main(){
     struct DL_Node *s;
     struct DL_Node *root;
     int i;
     root = malloc(sizeof(struct DL_Node));
     root->element = (char) NULL;
     root->next = NULL;
     root->prev = NULL;

     s = push(root, 'c');
     s = push(s, 'd');
     s = push(s, 'e');
     s = push(s, 'f');

     while (s)
          printf("%c\n", pop(&s));

}
0
 

Author Comment

by:raistlinx17
ID: 12122745
But isn't *n just a pointer? Can't I set is to whatever position (memory location) in the list I want and the calling code will have this new value? i.e Am I not passing by reference?
0
 
LVL 7

Expert Comment

by:aib_42
ID: 12122784
First things first:

1) main() returns int. Functions that take no parameters should be declared as taking 'void':
> void main(){
should be:
int main(void)

2) Always check the return value of malloc. It will be NULL if there is a problem with the memory allocation.

3) Casts.
> n->element = a;
will demote the int a to a char, but it should be no problem.
> root->element = (char) NULL;
NULL is used for pointer-context and its definition is compiler-dependent. The integer '0' will be converted to the null-pointer equivalent when used in a pointer-context, but the reverse is not true for the macro 'NULL'. Use 0 or '\0' instead. If you really want the three lines to look similar, use:
root->element = 0;
root->next = 0;
root->prev = 0;
Where the last two 0's will automatically be converted to the null pointer value used on your platform.

...and now to the problem:

pop() is freeing the structure alright, but you keep referring to the freed thing. Let's open up the while(){pop()} loop:
>while (s->prev)
>     printf("%c\n", pop(s));
Explode this to:

if (s->prev)
    printf("%c\n", pop(s));
if (s->prev)
    printf("%c\n", pop(s));
if (s->prev)
    printf("%c\n", pop(s));
if (s->prev)
    printf("%c\n", pop(s));
if (s->prev)
    printf("%c\n", pop(s));
.....

The first 'if' expression is returning true, and your first printf+pop pair is being executed. The pop() is freeing what s points to, and after that, you're referring to 'freed' memory, causing an undefined behvaiour. You should modify pop() so that it updates your pointer to point to the previous struct, now at the end of the stack. See these examples:

while(s->prev) s = pop(s, &some_char); /* pop returns the new top of the stack in its return value, and the 'element' of the previous top in some_char */

while(s->prev) printf("%c\n", pop(&s)); /* pop returns the element as usual, but takes a pointer to a pointer to struct DL_Node as a parameter. It first _reads_ s to free it, and then _writes_ to s the address of the new top struct */

or if you don't want to modify pop():

struct DL_Node *temp;

while(s->prev) {
    temp = s->prev;
    printf("%c\n", pop(s));
    s = temp;
}    
0
DevOps Toolchain Recommendations

Read this Gartner Research Note and discover how your IT organization can automate and optimize DevOps processes using a toolchain architecture.

 
LVL 5

Accepted Solution

by:
tzxie2000 earned 125 total points
ID: 12122827
for example
main()
{
 int *n;
 int a=1;
 n=&a;
 f(n);
 printf("%d",a);
}
 f(int * a)
 {
    a=2;
 }

you will get 2 as result

and when you call

 int a=1;
 int b=2;
main()
{
 int *n;
 n=&a;
 f(n);
 printf("%d",n==&b);
 printf("%d",n==&a);
}
 f(int * n)
 {
    n=&b;
 }
 
 you will get
 0//false
 1//true
 
 as you want to change the data store in n but not the data n point to
the data store in n is not changed
0
 
LVL 7

Expert Comment

by:aib_42
ID: 12122837
...what tzxie2000 wrote would be my second example. There were no other replies when I started writing mine, sorry :).
0
 
LVL 7

Expert Comment

by:aib_42
ID: 12122850
...what tzxie2000 wrote _the_first_time_ would be my second example. His other reply wasn't here when I started writing my last reply, sorry :).
0
 
LVL 5

Expert Comment

by:tzxie2000
ID: 12122957
and suggest a few code link stack below

struct DL_Node{
     char element;
     struct DL_Node *next;
};

struct DL_Node *push(struct DL_Node *top, int a){
    struct DL_Node *p;

    p = malloc(sizeof(struct DL_Node));
    p->next=top;
    p->element = a;
    return top;
}

char pop(struct DL_Node **top)
{
                  struct DL_Node *n;
                  char a;
                  
             if(!(*top)->next) return (char)(NULL);
             a=(*top)->element;
             n=(*top)->next;
     free(*top);
     *top=n;
     return a;
}

int isempty(struct DL_Node *top)
{
      return top->next;
}

void main(){
     struct DL_Node *top;
     int i;
     top = malloc(sizeof(struct DL_Node));
     top->element = (char) NULL;
     top->next = NULL;

     top = push(top, 'c');
     top = push(top, 'd');
     top = push(top, 'e');
     top = push(top, 'f');

     while (!isempty(top))
          printf("%c\n", pop(top));
}
0

Featured Post

Enterprise Mobility and BYOD For Dummies

Like “For Dummies” books, you can read this in whatever order you choose and learn about mobility and BYOD; and how to put a competitive mobile infrastructure in place. Developed for SMBs and large enterprises alike, you will find helpful use cases, planning, and implementation.

Question has a verified solution.

If you are experiencing a similar issue, please ask a related question

Have you thought about creating an iPhone application (app), but didn't even know where to get started? Here's how: ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ ~ Important pre-programming comments: I’ve never tri…
Preface I don't like visual development tools that are supposed to write a program for me. Even if it is Xcode and I can use Interface Builder. Yes, it is a perfect tool and has helped me a lot, mainly, in the beginning, when my programs were small…
The goal of this video is to provide viewers with basic examples to understand and use pointers in the C programming language.
The goal of this video is to provide viewers with basic examples to understand and use structures in the C programming language.

911 members asked questions and received personalized solutions in the past 7 days.

Join the community of 500,000 technology professionals and ask your questions.

Join & Ask a Question

Need Help in Real-Time?

Connect with top rated Experts

20 Experts available now in Live!

Get 1:1 Help Now