Reverse Dynamic Stack

Question

Could someone help me with this pile? The goal is to create a function that reverses the values of the stack, I tried to do this using the Invert() function of the code below, but it is not working, when I run it simply closes the code. In case anyone has any idea, I might be making the wrong allocation...

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

typedef struct elemento{
    int valor;
    struct Elemento *anterior;
} Elemento;

typedef struct pilha{
    struct Elemento *topo;
    int tam;
} Pilha;

Pilha *criaPilha(){
    Pilha *p = (Pilha*)malloc(sizeof(Pilha));
    p->topo = NULL;
    p->tam = 0;
    return p;
}
int vazia(Pilha *p){
    if(p->topo == NULL){
    return -1;
    } else {
    return 2;
    }
}
void empilha(Pilha * p, int v){
Elemento *e = (Elemento*)malloc(sizeof(Elemento));
e->valor = v;
    if(vazia(p) == 2){
        e->anterior = p->topo;
    }
    p->topo = e;
    p->tam++;
}
int desempilha(Pilha *p){
    int val;
    if(vazia(p) == 2){
        Elemento *aux;
        aux = p->topo; //Crio uma variável para ajudar a acessar o elemento anterior armazenado no topo;
        val = aux->valor;
        p->topo = aux->anterior; //Topo passa a ser o elemento anterior
        p->tam--;
        free(aux);
        return val;
    }
}

void maiorMenorMedia(Pilha *p, int *maior, int *media,int *menor){
int r = 0, cont = 0; 
int soma = 0;
int menorc = 99999;
int maiorc = -99999;
while(p->topo != NULL){
    r = desempilha(p);
    if(r > maiorc){
        maiorc = r;
    }
    if(r < menorc){
        menorc = r;
    }
    cont++;
    soma += r;
}
*maior = maiorc;
*media = (soma/cont);
*menor = menorc;
}

int maisElementos(Pilha *a, Pilha *b){
if(a->tam > b->tam){
    return 1;
} else {
    return 0;
}
}
Pilha * inverter(Pilha *pilha){
    int value;
    Pilha * aux = criaPilha();
    while(pilha->topo != 0){
        value = desempilha(pilha);
        empilha(aux, value);
    }
    return aux;
}

int main(int argc, char** argv) {
    Pilha *pteste = criaPilha();
    empilha(pteste, 15);
    empilha(pteste, 16);
    empilha(pteste, 17); 
    printf("%d\n", pteste->tam);

    int maior, media, menor;

    maiorMenorMedia(pteste, &maior, &media, &menor);
    printf("Maior: %d\n", maior);
    printf("menor: %d\n", menor);
    printf("media: %d\n", media);

Pilha *a = criaPilha();
Pilha *b = criaPilha();
empilha(a, 15);
empilha(a, 16);
empilha(b, 15); 
empilha(b, 16);
empilha(b, 17);
empilha(b, 18);

if(maisElementos(a,b) == 1){
    printf("Pilha A tem mais elementos\n");
}else{
    printf("Pilha B tem mais elementos\n");
}

Pilha *z = criaPilha();
z = inverter(b);
int x = 0;
x = desempilha(z);
printf("%d", x);
}

Answer 1

There are several details that are not right although the logic itself is right. I always advise to look with close attention for the compiler warnings as they are almost always errors and things that should correct.

• In defining the structure elemento:

  typedef struct elemento{
      int valor;
      struct Elemento *anterior;
      //     ^---
  } Elemento;
  

  The typedef is only finalized at the end of the instruction so you cannot use the name defined by it in the middle of the structure. In addition the typedef is designed to use only Elemento and not struct Elemento. That’s why the right thing is:

  typedef struct elemento{
      int valor;
      struct elemento *anterior;
      //     ^---
  } Elemento;
  

• Right in the next structure has the same problem:

  typedef struct pilha {
      struct Elemento *topo;
  //      ^-----^
      int tam;
  } Pilha;
  

  In which struct Elemento should only be Elemento 'Cause that’s what typedef who did earlier.

• The empilha does not set the last element to point to NULL and so the test if the battery is doing in function vazia, the if(p->topo == NULL) { does not function as expected and starts to access values outside the memory that declared.

  To correct just affect the e->anterior = NULL when it is the first Stack element:

  void empilha(Pilha * p, int v) {
      Elemento *e = (Elemento*)malloc(sizeof(Elemento));
      e->valor = v;
      if(vazia(p) == 2) {
          e->anterior = p->topo;
      }
      else { //se está vazia porque é o primeiro
          e->anterior = NULL; //mete esse a apontar para NULL
      }
      p->topo = e;
      p->tam++;
  }
  

There are many other things to improve but I leave only the most important:

• Avoid using loose numbers (magicians) such as if (vazia(p) == 2). This becomes very difficult to read and easy to miss. You can use #define to define constants that facilitates.
• Define function returns for all possible paths. In your function desempilha if the stack is empty there is no return specified.