Segmentation Fault when running pop command on dynamic stack

Asked

Viewed 36 times

-2

I’m trying to implement a dynamic stack but the program ends in error when executing the pop(pop) pop function, returning a Segmentation Fault. I can’t understand why.. I appreciate the help.

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

typedef char pilha_item;

struct pilha_no_struct { 
  pilha_item item;
  struct pilha_no_struct* prox;
};
typedef struct pilha_no_struct* pilha_no;

struct pilha_struct {
    unsigned quantidade;
    pilha_no topo;
};
typedef struct pilha_struct* pilha;

pilha inicializa()
{
    pilha nova_pilha = (pilha) malloc(sizeof(pilha));

    if (!nova_pilha)
        return (NULL);

    nova_pilha->quantidade = 0;
    nova_pilha->topo = NULL;

    return nova_pilha;
}

int vazia(pilha p)
{
    return p->quantidade == 0;
}

int empilha(pilha p, pilha_item item)
{
    if (vazia(p))
    {
        pilha_no novo_no = (pilha_no)malloc(sizeof(pilha_no));

        if (!novo_no)
            return (0);

        novo_no->item = item;
        novo_no->prox = NULL;

        p->topo = novo_no;
        p->quantidade++;

        return (1);
    }

    else
    {
        pilha_no novo_no = (pilha_no)malloc(sizeof(pilha_no));

        if (!novo_no)
            return (0);

        novo_no->item = item;
        novo_no->prox = p->topo;
        p->topo = novo_no;
        p->quantidade++;

        return (1);
    }
}

void elements(pilha p){
    
    while(p->topo){
        printf("%c", p->topo->item);
        p->topo = p->topo->prox;
    }
}
 
int desempilha(pilha p)
{
    if (vazia(p))
        return (0);

    else
    {   
        if(p->quantidade == 1){
            pilha_no temp = p->topo;
            p->topo = NULL;
            free(temp);
            return (1);
        }
        else{
            pilha_no temp = p->topo;
            p->topo = p->topo->prox;
            free(temp);
            return (1);
        }

    }
}


int main()
{
    pilha p = inicializa();
    empilha(p, 'S');
    empilha(p,'N');
    empilha(p, 'S');
    elements(p);
    desempilha(p);
    return 0;
}
  • Your program is a little confused, I found. Only in the first 20 lines appears an impressive amount of pile-it stack-it. I think you should not declare --- typedef --- a pointer as a stack. It only causes confusion. And error. See this line: pilha nova_pilha = (pilha)malloc(sizeof(pilha)); It has 4 times the word stack. And it’s allocating to the :D stack the area of a pointer... That’s not what you want...

  • I understand what you mean! But this is a college exercise that my teacher went through. He gave the structures already with the typedefs and wants the implementation of these functions.

  • I’ll show you an example with your code and tell me if you understand what I meant... you got the line error I was talking about?

1 answer

0

I changed your code a little to show you what I was talking about

Its structure

typedef char pilha_item;

struct pilha_no_struct {
    pilha_item item;
    struct pilha_no_struct* prox;
};
typedef struct pilha_no_struct* pilha_no;

struct pilha_struct {
    unsigned quantidade;
    pilha_no topo;
};
typedef struct pilha_struct* pilha;

It is a stack, a stack has nodes that contain items. In your case you’re using a fine print, but it could be a pointer to a complex structure

consider for example the same thing written like this

typedef char Item;

typedef struct _node_
{
  Item              item;
  struct    _node_* prox;
} Node;

typedef struct
{
    unsigned    quantidade;
    Node*       topo;
}   Pilha;

Where a Pilha has Node that has Item.

Declare your duties AFTER main() and leave only the prototypes in front, or still use a file. h and separate everything. It is better for you and better for those who will read your program. It’s boring to be looking for the function in the middle of the code.

And adopt some convention to identify your types. I used as an example the first letter capitalized.

About the functions, why didn’t you use the classic names, push() pop() top() empty() size() ?

an example

Consider this program:

int main()
{
    Pilha* pilha = inicializa();
    elements(pilha);
    for (char item = 'a'; item <= 'z'; item += 1)
        pilha = empilha(&item, pilha);
    elements(pilha);
    while (!vazia(pilha))
    {   mostra(pilha);
        desempilha(pilha);
    };
    printf("\nPilha ao final:\n");
    elements(pilha);
    return 0;
}

Who will do as is there, create such a pile, put the 26 little letters, take one by one and close. I didn’t write the function that erases the stack, but it’s trivial.

the result

Pilha tem 0 elementos


Pilha tem 26 elementos
z y x w v u t s r q
p o n m l k j i h g
f e d c b a

Topo => 'z' [26 elementos]
Topo => 'y' [25 elementos]
Topo => 'x' [24 elementos]
Topo => 'w' [23 elementos]
Topo => 'v' [22 elementos]
Topo => 'u' [21 elementos]
Topo => 't' [20 elementos]
Topo => 's' [19 elementos]
Topo => 'r' [18 elementos]
Topo => 'q' [17 elementos]
Topo => 'p' [16 elementos]
Topo => 'o' [15 elementos]
Topo => 'n' [14 elementos]
Topo => 'm' [13 elementos]
Topo => 'l' [12 elementos]
Topo => 'k' [11 elementos]
Topo => 'j' [10 elementos]
Topo => 'i' [ 9 elementos]
Topo => 'h' [ 8 elementos]
Topo => 'g' [ 7 elementos]
Topo => 'f' [ 6 elementos]
Topo => 'e' [ 5 elementos]
Topo => 'd' [ 4 elementos]
Topo => 'c' [ 3 elementos]
Topo => 'b' [ 2 elementos]
Topo => 'a' [ 1 elementos]

Pilha ao final:

Pilha tem 0 elementos

Then we go back to that show.

In your program

pilha nova_pilha = (pilha)malloc(sizeof(pilha));

This is the biggest problem in your code because pilha is a pointer so should have written sizeof(*pilha) considering your statement

typedef struct pilha_struct* pilha;

stack()

The code to stack() is very complicated

    if (vazia(p))
    {
        pilha_no novo_no = (pilha_no)malloc(sizeof(pilha_no));

        if (!novo_no)
            return (0);

        novo_no->item = item;
        novo_no->prox = NULL;

        p->topo = novo_no;
        p->quantidade++;

        return (1);
    }

    else
    {
        pilha_no novo_no = (pilha_no)malloc(sizeof(pilha_no));

        if (!novo_no)
            return (0);

        novo_no->item = item;
        novo_no->prox = p->topo;
        p->topo = novo_no;
        p->quantidade++;

        return (1);
    }
}

Why do you use an Else after Return? It only makes reading difficult. A Return is definitive. There is no reason for an Else.

And it’s wrong to create something in empilha() when the battery is empty: just call inicializa() right there. If you change anything in inicializa() always left that "loose end" there.

See how it can be simpler

Pilha*  empilha(Item* item, Pilha* p)
{
    if (vazia(p)) p = inicializa();
    Node* novo = (Node*)malloc(sizeof(Node));
    novo->item = *item;
    novo->prox = p->topo;
    p->quantidade += 1;
    p->topo = novo;
    return p;
};  // empilha()

and the other way around:

int     desempilha(Pilha* p)
{
    if (vazia(p)) return (0);
    Node* segundo = p->topo->prox;
    free(p->topo);
    p->topo = segundo;
    p->quantidade -= 1;
    return p->quantidade;
};  // desempilha()

back to the example

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


typedef char Item;

typedef struct _node_
{
  Item              item;
  struct    _node_* prox;
} Node;

typedef struct
{
    unsigned    quantidade;
    Node*       topo;
}   Pilha;

int     desempilha(Pilha*); // tira um POP()
int     elements(Pilha*); // lista todos
Pilha*  empilha(Item*, Pilha*); // poe um PUSH()
Pilha*  inicializa(); // cria
Item*   mostra(Pilha*); // mostra o topo TOP()
int     vazia(Pilha*); // empty()
int     size(Pilha*); // size()

int main()
{
    Pilha* pilha = inicializa();
    elements(pilha);
    for (char item = 'a'; item <= 'z'; item += 1)
        pilha = empilha(&item, pilha);
    elements(pilha);
    while (!vazia(pilha))
    {   mostra(pilha);
        desempilha(pilha);
    };
    printf("\nPilha ao final:\n");
    elements(pilha);
    return 0;
}

int     desempilha(Pilha* p)
{
    if (vazia(p)) return (0);
    Node* segundo = p->topo->prox;
    free(p->topo);
    p->topo = segundo;
    p->quantidade -= 1;
    return p->quantidade;
};  // desempilha()

int     elements(Pilha* pilha)
{
    int col = 0;
    printf("\nPilha tem %d elementos\n", size(pilha));
    Node* p = pilha->topo;
    while (p != NULL)
    {
        printf("%c ", p->item);
        col += 1;
        if (col % 10 == 0) printf("\n");
        p = p->prox;
    };  // while()
    if (col % 10 != 0) printf("\n");
    printf("\n");
    return pilha->quantidade;
};

Pilha*  empilha(Item* item, Pilha* p)
{
    if (vazia(p)) p = inicializa();
    Node* novo = (Node*)malloc(sizeof(Node));
    novo->item = *item;
    novo->prox = p->topo;
    p->quantidade += 1;
    p->topo = novo;
    return p;
};  // empilha()

Pilha* inicializa()
{
    Pilha* pilha = (Pilha*)malloc(sizeof(Pilha));
    if (pilha == NULL) return (NULL);
    pilha->quantidade = 0;
    pilha->topo = NULL;
    return pilha;
};

Item* mostra(Pilha* p)
{
    if (p == NULL) return NULL;
    if (p->topo == NULL) return NULL;
    printf("Topo => '%c' [%2d elementos]\n",
        p->topo->item, p->quantidade);
    return NULL;
};

int     size(Pilha* p)
{
    return p->quantidade;
}

int     vazia(Pilha* p)
{
    return p->quantidade == 0;
}

Compare with this code. Maybe you find it more readable. I typed on top of your code

  • Thank you very much for the answer, but it escapes the question somewhat. The nomenclature that has been used in the structures cannot be changed. This is an exercise that my teacher passed, so the name of the functions as well as the definition of the structure cannot be changed. So I didn’t use more common nameplates like push and pop. The only thing I can change is the implementation of each function. Thanks for pointing out Else after Return, I’ll change it so it’s a little clearer. I tried to switch to sizeof(*stack), but the compiler is error while the way it is in the program works

  • From what I’ve tested, the stacking function is working properly, despite presenting the clarity issues you pointed out. The error occurs on this line of the pop-up function: p->top = p->top->Prox; which generates the Segmentation fault error.

  • It’s wrong. Which compiler are you using? needs to allocate the right size. compared the two programs? Are you allocating sizeof(pilha) which is 4 or 8. Understand this. So it will cancel sooner or later. You have to allocate the right size. If *pilha is not good use sizeof(struct pilha_struct). I’ve told you three times it’s wrong. Use one printf() and you’ll see.

  • And compare the codes of your program with what I changed. Your routine elements() is mocking the pointers...

Browser other questions tagged

You are not signed in. Login or sign up in order to post.