c linux stack segmentation-fault towers-of-hanoi

Segmentation fault in stack in push Function C language

I had faced problem with it when i start the program it crashed at function Full and push line 70 and 78 (Linux). I try to fix it but I always crash at the same place.

   #include <stdio.h>
   #define SIZE 64

   struct stack{
      int TowerTop;
      int Elem[SIZE];
   };

   /* Forward Declarations */                    
   void push(int Elem, struct stack *x);
   int  create (struct stack *x);
   int  full(const struct stack *x);
   void pop(struct stack *x);
   int lolol(struct stack *x,struct stack *x2,struct stack *x3,int a);

   void main() {
      int j;
      int a = 5; 

      //scanf("%d",&a);

      struct stack Tower1;
      struct stack Tower2;
      struct stack Tower3;

      create(&Tower1);
      create(&Tower2);
      create(&Tower3);

      for(int i=0; i<=SIZE-1; i++) {
          Tower1.Elem[i]=0;
          Tower2.Elem[i]=0;
          Tower3.Elem[i]=0; 
      }

      for(int i=0; i<=a; i++){
          Tower1.Elem[i]=i+1;
      }

      // Display initial tower setup
      for(int i=0; i<a; i++){
         printf("%d %7d %7d\n",Tower1.Elem[i],Tower2.Elem[i],Tower3.Elem[i]);
      }

      lolol(&Tower1,&Tower2,&Tower3,a);

      // Display Tower after move made by lolol
      printf("%d\n", j);
      for(int i=0; i<a; i++) {
         printf("%d %7d %7d\n",Tower1.Elem[i],Tower2.Elem[i],Tower3.Elem[i]);
      }
    }

    int lolol(struct stack *x,struct stack *x2,struct stack *x3,int a) {
       if(a == 1) {
          printf("\n Disk 1 move to");
          push(x->Elem[x->TowerTop], x2->Elem[x2->TowerTop]); j++;
          pop(x->Elem[x->TowerTop]);
          return 0; 
       }

       lolol(x, x3, x2, a-1);
       push(x->Elem[x->TowerTop], x2->Elem[x2->TowerTop]); j++;
       pop(x->Elem[x->TowerTop]);
       lolol(x3, x2, x, a-1);
      }

      int create(struct stack *x) {
         x->TowerTop = -1;
      }

      int full(const struct stack *x) {
          if (x->TowerTop == SIZE-1) {
             return 1;
          } else {
             return 0;
          } 
       }

       void push(int Elem, struct stack *x){
          if(full(x)) {
             printf("Stack is full"); 
          } else {
             x->TowerTop++;
             x->Elem[x->TowerTop]=Elem;
          }
       }

       void pop( struct stack *x) {
          if(x->TowerTop== -1) {
             printf("Empty");
          } else {
             x->TowerTop--;
          }
       }

Solution

we will debug your code together. But first, I'll tell you more on how to ask questions on forums (thus on StackOverflow).

First there is no 'as fast as possible' since we're all here to help. We'll try our best and it may or may not be on time. Then please read your message again with your code and ask yourself, is this readable? I really do hope your code does not look the same way in your file and you just failed somehow the copy/paste because this code is quite unreadable.

First step: Change the code to make it readable.

I will skip the verbosity of this step but basically it is indenting, adding spaces when needed etc.

#include <stdio.h>

#define SIZE 64

struct stack
{
  int TowerTop;
  int Elem[SIZE];
};

void push(int Elem, struct stack *x);
void pop(struct stack *x);
int create (struct stack *x);
int full(const struct stack *x);
int lolol(struct stack *x, struct stack *x2, struct stack *x3, int a);

int a;
int j;

void main()
{

  //scanf("%d", &a);
  a = 5;
  struct stack Tower1;
  struct stack Tower2;
  struct stack Tower3;
  create(&Tower1);
  create(&Tower2);
  create(&Tower3);

  for (int i = 0; i <= SIZE - 1; i++)
    {
      Tower1.Elem[i] = 0;
      Tower2.Elem[i] = 0;
      Tower3.Elem[i] = 0;
    }

  for (int i = 0; i <= a; i++)
    {
      Tower1.Elem[i] = i + 1;
    }

  for (int i = 0; i < a; i++)
    {
      printf("%d %7d %7d\n", Tower1.Elem[i], Tower2.Elem[i], Tower3.Elem[i]);
    }

   lolol(&Tower1, &Tower2, &Tower3, a);

   printf("%d\n", j);

   for (int i = 0; i < a; i++)
    {
      printf("%d %7d %7d\n", Tower1.Elem[i], Tower2.Elem[i], Tower3.Elem[i]);
    }
}

int lolol(struct stack *x, struct stack *x2, struct stack *x3, int a)
{
  if (a == 1)
    {
      printf("\n Disk 1 move to");
      push(x->Elem[x->TowerTop], x2->Elem[x2->TowerTop]);
      j++;
      pop(x->Elem[x->TowerTop]);
      return 0;
    }
  lolol(x, x3, x2, a - 1);
  push(x->Elem[x->TowerTop], x2->Elem[x2->TowerTop]);
  j++;
  pop(x->Elem[x->TowerTop]);
  lolol(x3, x2, x, a - 1);
}

int create(struct stack *x)
{
  x->TowerTop = -1;
}

int full(const struct stack *x)
{
  if (x->TowerTop == SIZE-1)
    {
      return 1;
    }
  else
    {
      return 0;
    }
}

void push(int Elem, struct stack *x)
{
  if (full(x))
    {
      printf("Stack is full");
    }
  else
    {
      x->TowerTop++;
      x->Elem[x->TowerTop] = Elem;
    }
}

void pop(struct stack *x)
{
  if (x->TowerTop == -1)
    {
      printf("Empty");
    }
  else
    {
      x->TowerTop--;
    }
}

Please not that I did not change any logic or types or whatever.

Second step: reading code.

Actually I'll probably come back to this later but I took a quick read and there is some basic stuff that need to be said. The main function shall always returns an int. According to the C standard, you can only declare the main function with two ways: int main(void) (takes no parameters) or int main(int argc, char *argv[]); (takes command line parameters).

Then, you must know that the two variables a and j are globals to the file. That mean you can access them everywhere in the same source file. Sounds good, doesn't work. You should NEVER EVER do global variables, unless you have NO OTHER CHOICE (e.g using sigaction). In your case, they are really not needed.

Lastly, COMMENTS. Please, commend your code. While most of it is understandable easily here, the function lolol (that name does NOT make sense at all btw) is doing something I don't understand when taking a quick look at the code.

Third step: compiling

gcc file.c => 4 warnings. Unless you know what you're doing and are an experienced C programmer - which you are not (yet!) you shouldn't have any warnings when compiling. Furthermore, you should compile with the -Wall -Wextra switches. That will throw more warnings but allow you to catch bugs before they even exists.

So we will fix the warnings:

warning: return type of ‘main’ is not ‘int’ [-Wmain]. Alright, let's switch that. void main() becomes int main(void), as I said earlier.
file.c:64:34: warning: passing argument 2 of ‘push’ makes pointer from integer without a cast [-Wint-conversion] => Alright, here you're using the push function which I'm assuming you wrote. However, this function expects an int as it's first parameter and a struct stack * as it's second parameter when you're passing it an int. Let's fix that: push(x->Elem[x->TowerTop], x2->Elem[x2->TowerTop]); becomes push(x->Elem[x->TowerTop], x2);, since you're pushing x->Elem[x->TowerTop] on the top of the x2 stack.
Same thing for the next three warnings, being the pop line 66, the push line 70, and the pop line 72.
Next two warnings are the same: file.c:74:1: warning: control reaches end of non-void function [-Wreturn-type] => I'm still not sure what lolol is meant to do, so I'll just add a return 0 at the end of it, and switch the create function to void type.

Here's the corrected code:

#include <stdio.h>

#define SIZE 64

struct stack
{
  int TowerTop;
  int Elem[SIZE];
};

void push(int Elem, struct stack *x);
void pop(struct stack *x);
void create (struct stack *x);
int full(const struct stack *x);
int lolol(struct stack *x, struct stack *x2, struct stack *x3, int a);

int a;
int j;

int main(void)
{

  //scanf("%d", &a);
  a = 5;
  struct stack Tower1;
  struct stack Tower2;
  struct stack Tower3;
  create(&Tower1);
  create(&Tower2);
  create(&Tower3);

  for (int i = 0; i <= SIZE - 1; i++)
    {
      Tower1.Elem[i] = 0;
      Tower2.Elem[i] = 0;
      Tower3.Elem[i] = 0;
    }

  for (int i = 0; i <= a; i++)
    {
      Tower1.Elem[i] = i + 1;
    }

  for (int i = 0; i < a; i++)
    {
      printf("%d %7d %7d\n", Tower1.Elem[i], Tower2.Elem[i], Tower3.Elem[i]);
    }

   lolol(&Tower1, &Tower2, &Tower3, a);

   printf("%d\n", j);

   for (int i = 0; i < a; i++)
    {
      printf("%d %7d %7d\n", Tower1.Elem[i], Tower2.Elem[i], Tower3.Elem[i]);
    }
}

int lolol(struct stack *x, struct stack *x2, struct stack *x3, int a)
{
  if (a == 1)
    {
      printf("\n Disk 1 move to");
      push(x->Elem[x->TowerTop], x2);
      j++;
      pop(x);
      return 0;
    }
  lolol(x, x3, x2, a - 1);
  push(x->Elem[x->TowerTop], x2);
  j++;
  pop(x);
  lolol(x3, x2, x, a - 1);
  return 0;
}

void create(struct stack *x)
{
  x->TowerTop = -1;
}

int full(const struct stack *x)
{
  if (x->TowerTop == SIZE-1)
    {
      return 1;
    }
  else
    {
      return 0;
    }
}

void push(int Elem, struct stack *x)
{
  if (full(x))
    {
      printf("Stack is full");
    }
  else
    {
      x->TowerTop++;
      x->Elem[x->TowerTop] = Elem;
    }
}

void pop(struct stack *x)
{
  if (x->TowerTop == -1)
    {
      printf("Empty");
    }
  else
    {
      x->TowerTop--;
    }
}

Compiling and running without SIGSEGV.

Last step: Going further

I'll talk about topics more advanced, but it could be useful to you. First, get more informations about stack and how they works because I'm not sure you've completely understood it.

Also, to debug your programs you can use valgrind, it is a memory checker tool that is very useful and always comes in a handy. To use it, add the flag -g when compiling.

I suggest looking into gdb as well, the GNU Linux debugger. Very powerful.

The function full can be changed to a simple return x->TowerTop == SIZE - 1. It avoids branching and hence will run faster.

And that's all folks. Still not sure what your code should do but you only asked about how to fix your segfault. Now please just remember. Do not let any warnings when compiling.

Thank you.