Stack C++. One element not displayed

Question

The program reads a text file with the contents

A  DPX 

A  QRT

Pushes DPX and QRT on stack. But while displaying stack{void show()}, it does not display stack[0]. No idea Why!? It is being pushed onto the stack. element stack[0] diplayed in push() but not in show()

const int MAX = 10 ;
struct car
{
        char * lPlate;
        int moves;
};

class garage
{
    private :

        char * stack[MAX] ;
        int top ;

    public :
            garage( )
            {
               top = -1 ;
            }

        void arrive(struct car  c)
         {
            push(c.lPlate);
         }

        void depart(struct car c )
         {

         }

        void push ( char * item ) ;
        char* pop( ) ;
        void show();
} ;

void garage:: push ( char* item )
{
    if ( top == MAX - 1 )
         cout << endl << "Sorry. Parking lot is full" ;
    else
    {
        top++ ;
        strcpy(stack[top] , item) ;
    }
    cout<<"In push: "<<stack[top];
}

char * garage:: pop( )
{
  if ( top == -1 )
    {
        cout << endl << "Parking lot empty is empty" ;
        return NULL ;
    }

    return stack[top--] ;
}

void garage:: show()
{
   cout<<" \nParking Lot Status:\n";
   if( top == -1 )
    {
        cout << endl << "Parking lot is empty" ;
        return ;
    }
    int i=top;
    cout<<"In show "<<stack[0]<<endl;
    while(i>-1)
    {
     cout<<stack[i]<<endl;  i--;
    }
}
int main()
{
    clrscr();
    char *action;
    garage  g;
    ifstream fin("TEST");
    if(!fin){cout<<"Cannot open i/p file \n\n";return 1;}
    char str1[80],str2[40]; int i,j;
    while(!fin.eof())
    {
     fin.getline(str2,MAX);
     struct car c;//create a car obj
     char * pch;
     pch = strtok (str2," ,.-");//split string on getting <space>/<,>/<.>/<->
    while (pch != NULL)
    {
      action=pch;

     pch = strtok (NULL, " ,.-");
     c.lPlate=pch;
     pch=NULL;
     cout<<"\nAction: "<<action<<" lPlate: "<<c.lPlate<<"\n";
      switch(*action)
      {
        case 'A':
            g.arrive(c);
            break;
        case 'D':
            g.depart(c);
            break;
        }
    }
    cout<<"\n\n";
    }

        fin.close();
        cout<<"\n";
        g.show();
    getch();
}

used. everything's working fine. just stack[0] not displayed — Coder, Sep 11 '16 at 18:40
It's too long. Follow [this](http://stackoverflow.com/help/mcve) and pay attention to the last link, [How to debug small programs](http://ericlippert.com/2014/03/05/how-to-debug-small-programs/) (Good luck! :-) — Amit, Sep 11 '16 at 18:42
You have an array of pointers. You never initialize the pointers. Then you use the pointers to write where they point. Undefined behavior. — Some programmer dude, Sep 11 '16 at 18:43
I see many char pointers there, but no `new` or `malloc`. Are you allocating memory somewhere? — Adrian Jałoszewski, Sep 11 '16 at 18:43
no. don't work on depart(). just work on arrive() and push() and show().ignore depart() pls — Coder, Sep 11 '16 at 18:43
char pointer is to hold the addr of the string. it works like a char array — Coder, Sep 11 '16 at 18:44
Why didn't you just use `std::string` instead of the `char *` stuff? — PaulMcKenzie, Sep 11 '16 at 18:45
Yes the pointers holds the addresses to where the strings should be. The problem is that you never actually make them *point* anywhere. The pointers will be *indeterminate* and you will have *undefined behavior*. I'm surprised the code doesn't crash actually, as the `strcpy` call will write to seemingly random memory locations. — Some programmer dude, Sep 11 '16 at 18:45
I am. In the `push` function you use uninitialized pointers. End of story. — Some programmer dude, Sep 11 '16 at 18:47
I'm teaching basic c++. no std::string. only array of characters — Coder, Sep 11 '16 at 18:47
Umm `std::string` **is** basic C++. It has been for the past 18 years. What you're "teaching" is actually C programming. — PaulMcKenzie, Sep 11 '16 at 18:48
but i am passing c.lPlate to push. its initialised by the passed parameter — Coder, Sep 11 '16 at 18:48
std::string why doesnt it work on turbo C++ v3 then? needs something? — Coder, Sep 11 '16 at 18:49
The problem is the `stack` array. You never initialize it, so its contents is indeterminate. You never make the pointers in `stack` point anywhere. You must allocate memory for them to point to. — Some programmer dude, Sep 11 '16 at 18:49
@Coder `turbo C++ v3` -- Why are you using this dinosaur from 20 years ago? Yes, there is something you need -- an up to date, modern, C++ compiler, something from this generation. — PaulMcKenzie, Sep 11 '16 at 18:51
@Paul : i tried installing visual c++. it says successfully installed. but not found anywhere in the system — Coder, Sep 11 '16 at 18:52
@joachim: if stack array is the problem, how come all except stack[0] displayed? — Coder, Sep 11 '16 at 18:53
[Learn how `strtok` works](http://en.cppreference.com/w/cpp/string/byte/strtok). In your input loop, `c.lPlate=pch;` - that makes your member pointer `lPlace` in your instance `c` point to an address within the buffer `str2`. That buffer will be reused on the next iteration of the loop. loaded with different data and sent through another `strtok` series. Anything that had pointers into that buffer are going to be pointing to what was *last* read. Now think what that means to the rest of your code. — WhozCraig, Sep 11 '16 at 19:09
@Coder *if stack array is the problem...* -- It **is** the problem. Your code invokes undefined behavior as soon as this line is executed: `strcpy(stack[top], item);` -- all due to `stack[top]` being uninitialized. Anything you see happening after that line is pure luck that it is even occurring. — PaulMcKenzie, Sep 11 '16 at 19:12
@Coder turbo c++ development ended in 94 and the brief 2006-2009 revival did not introduce support for much of C++98 never mind C++03, C++11 or C++14. What you are writing is not C++ but C with classes, as evidenced by `struct car c`. Your problem is caused by slogging away at problematic C functionality that C++ has long since addressed. Also http://stackoverflow.com/questions/5605125/why-is-iostreameof-inside-a-loop-condition-considered-wrong — kfsone, Sep 11 '16 at 19:12
From PaulMcKenzie: "It has been for the past 18 years." From Coder: "std::string why doesnt it work on turbo C++ v3 then?" Turbo C++ V3 hit the market in 1991. 1991 + 18 = 2009. The current year is 2016. Your compiler is older than standardized C++ (1998). The language has changed lot in the past 25 years. I won't say it's useless to learn from, but you'll have a hard time finding work in a shop that doesn't use 25 year old technology. — user4581301, Sep 11 '16 at 19:16
@Coder its also worth pointing out that a real stack only provides access, addition or removal at the top. What you are implementing is closest to a vector, although a std::map could be a good match for your key (lplate) based lookup. — kfsone, Sep 11 '16 at 19:17
Also, can someone suggest a simple C++ compiler. Latest one... — Coder, Sep 12 '16 at 04:48

score 0 · Accepted Answer · edited May 23 '17 at 12:22

The array

char * stack[MAX] ;

is not initialised. It is an array of 10 (MAX) pointers. Each pointer is not initialised.

In the method pop, before copying the item to the stack, initialise the "top" item of the stack

top++ ;
int length = strlen(item)+1; //+1 for null terminating character
stack[top] = new char[length]; //allocate memory
strncpy(stack[top] , item, length); //prefer strncpy

After reading the file

A  DPX 
A  QRT

This will result in an array which looks something like this

Note

Gray items remain uninitialised.
Avoid strcpy because it can cause an overflow. If you must use pointers prefer strncpy.
Memory allocated to the stack now needs to be correctly deallocated, e.g.
- in the pop method make a copy of the item popped off the stack and delete stack[job] or /and
- in the deconstructor free the memory that has not been popped.

Change the pop method to something like this

int length = strlen(stack[top])+1;
char *returnItem = new char[length];
strncpy(returnItem,stack[top],length);
delete stack[top];
top--;
return returnItem;

Who ever calls the pop method needs to deallocate the memory for "returnItem"

In the deconstructor free all memory still held by the stack array, i.e.

  ~garage() {
    for (int i=top; i>=0; i--) {
      std::cout << "clean up" << i << std::endl;
      delete stack[i]; 
     }
  }

Thanks. Uninitializing itself was the problem. Luckily for others except stack[0], it somehow worked. Now, after initializing, its working fine. Whoo!! — Coder, Sep 12 '16 at 04:47

Stack C++. One element not displayed

1 Answers1