0

I have a stack using structure. I need to return a string while i pop. so i try to copy the string to a pointer using strcpy(), but when i run the program, the program stops working right at that step.

Here's the code for stack.

struct node{            // stack structure
    char data[5];
    struct node *link;
}*top=NULL;

Here's the code for pop function.

char* pop(){
    printf("\nIn pop fun.");
    if(top==NULL)
    {
        printf("Error!!!\nStack Underflow.");
        return "error";
    }
    printf("\nChecked if pop is null.");
    char *popvar;
    printf("\nCreated new char pointer.");
    strcpy(popvar,top->data);
    printf("\nCopied data from top.");
    struct node *tmp = top;
    printf("\nCreated new node.");
    top=tmp->link;
    printf("\n Assigned top new value.");
    free(tmp);
    printf("\nFree temp");
    printf("\npoped from stack.");
    return popvar;
}

Anyone please help...

Support Ukraine
  • 42,271
  • 4
  • 38
  • 63
Nipun Shaji
  • 5
  • 1
  • 3
  • `char *popvar;` does not allocate memory to hold the string. Use `char *popvar = malloc(5);` and don't forget to `free` it after its use – Spikatrix Sep 21 '18 at 09:56
  • The usual trick concerning stacks: Access data at top before pop. Otherwise, if you want to `strcpy()` a string you have to provide sufficient storage for destination. – Scheff's Cat Sep 21 '18 at 09:56
  • `char *popvar;` ==> `char *popvar = malloc(5)` In the code that called `pop` you need to `free` the memory once your done with the string – Support Ukraine Sep 21 '18 at 10:04

2 Answers2

0

Dynamically allocated memory belongs to the program and exists even when scope ends.

Only the memory which have been dynamically allocated can exist after the scope has ended, once your function ends, the pointer popvar ends as well but not if:

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

 char *fun()
 {
    char *c=malloc(10);
    c[0]='h';
    c[1]='e';
    c[2]='l';
    c[3]='l';
    c[4]='\0';

    return c;
}

int main(){

    printf("%s",fun());

    return 0;   
}

Copy that popped data into a dynamically allocated memory and then the memory can be accessed from outside of that pop function. Plus before copying it with strcpy you didn't allocate memory in which the popped value is to be copied.

Gaurav
  • 1,570
  • 5
  • 17
  • Hmm... your answer is mostly correct but also a bit wrong. It's correct that `popvar` goes out of scope when the function returns but that irrelevant as the value of `popvar` is actually returned. The real problem is that no memory was allocated so `popvar` doesn't point to legal memory and therefore `strcpy` is illegal. BTW: you should add a call of `free` to your program – Support Ukraine Sep 21 '18 at 10:11
  • @4386427 I was just mentioning that :) – Gaurav Sep 21 '18 at 10:14
0

You can't write, via strcpy() or otherwise, to an uninitialized pointer. That's writing to an undefined memory address, so the behavior is undefined.

It would be legal if you'd declared an array to strcpy() into:

char popvar[5];
strcpy(popvar, top->data);

Or a struct node, which has an array (not pointer) member:

struct node popvar;
strcpy(popvar.data, top->data);

You can't return these values to the caller of pop() without copying them again, though. For that, you can allocate dynamic (heap) memory:

char *popvar = malloc(5);
strcpy(popvar, top->data);
top = top->link;
return popvar;

In this case the caller must always remember to call free() on this result. Every malloc() must be eventually followed by a free(), otherwise you have a memory leak. Note that your original program calls free() without ever having called malloc(); that is illegal and its behavior is undefined.

Another possibility is to require the caller to decide how to store the result:

void pop(char *result) {
    strcpy(result, top->data);
    top = top->link;
}

This function would allow either of the following usages:

char str[5];
pop(str);

Or:

char *str = malloc(5);
pop(str);
/* do stuff */
free(str);

Or even:

struct {
    int foo;
    int bar;
    char other_data[5];
} some_other_structure;
pop(some_other_structure.other_data);
Nathan Dorfman
  • 331
  • 1
  • 3