-4

Hello everybody I am not able to allocate memory in c++ using the new operator.The code is as below

#include <iostream>
#include<stack>
#include<queue>
#include<string>

using namespace std;

class cell
{
    public:
    unsigned short int x, y;
    unsigned short int dis;
    cell* parent;
    string step;
    cell(){}
    cell(unsigned short int x, unsigned short int y, cell* parent, unsigned short int d, string s):x(x), y(y), parent(parent), dis(d), step(s){}
};

bool isinside(int i, int j, int n)
{
    if(i >= 0 && i < n && j >= 0 && j < n)
        return true;
    return false;
}

void printShortestPath(unsigned short int n, unsigned short int i_start, unsigned short int j_start, unsigned short int i_end, unsigned short int j_end)
{
    //  Print the distance along with the sequence of moves.
   // cout << "safe1.1";
    queue<cell*> q;
    bool** vis = new bool*[n+1];
    cout << "safe1.2\n";
    for(unsigned short int i = 0; i < n+1; i++)
        *(vis+i) = new bool;
    cout << "safe1.3\n";
    for(unsigned short int i = 0; i < n; i++)
        for(unsigned short int j = 0; j < n; j++)
            vis[i][j] = false;
    cout << "safe1.5\n";
    cell* c;
    q.push(c = new cell(i_start, j_start, NULL, 0, "0"));   //**ERROR HERE**
    cout << "safe1.4\n";
    bool reached = false;
    cell* t;
    cout << "safe2\n";
    while(!q.empty())
    {
        t = q.front();
       // cout << t->x << " " << t->y << " " << t->step << " " << t << endl;
        q.pop();
        vis[t->x][t->y] = true;
        cout << t << " ";
        if(t->x == i_end && t->y == j_end)
        {
            reached = true;
            break;
        }
        if(isinside(t->x-2,t->y-1,n) && !vis[t->x-2][t->y-1])
        {
            q.push(new cell(t->x-2,t->y-1,t,t->dis+1, "UL"));
            vis[t->x-2][t->y-1] = true;
        }
        if(isinside(t->x-2,t->y+1,n) && !vis[t->x-2][t->y+1])
        {
            q.push(new cell(t->x-2,t->y+1,t,t->dis+1, "UR"));
            vis[t->x-2][t->y+1] = true;
        }
        if(isinside(t->x,t->y+2,n) && !vis[t->x][t->y+2])
        {
            q.push(new cell(t->x,t->y+2,t,t->dis+1,"R"));
            vis[t->x][t->y+2] = true;
        }
        if(isinside(t->x+2,t->y+1,n) && !vis[t->x+2][t->y+1])
        {
            q.push(new cell(t->x+2,t->y+1,t,t->dis+1,"LR"));
            vis[t->x+2][t->y+1] = true;
        }
        if(isinside(t->x+2,t->y-1,n) && !vis[t->x+2][t->y-1])
        {
            q.push(new cell(t->x+2,t->y-1,t,t->dis+1,"LL"));
            vis[t->x+2][t->y-1] = true;
        }
        if(isinside(t->x,t->y-2,n) && !vis[t->x][t->y-2])
        {
            q.push(new cell(t->x,t->y-2,t,t->dis+1,"L"));
            vis[t->x][t->y-2] = true;
        }
        cout << t << endl;
    }
    cell* root = t;
    if(reached)
    {
        cout << "safe3\n";
        stack<string> st;
        unsigned short int aa = 0;
        while(root != NULL)
        {
            aa++;
            st.push(root->step);
         //   cout << "root " << root << endl;
            root = root->parent;
        }
        cout << aa-1 << endl;
        st.pop();
        while(!st.empty())
        {
            cout << st.top() << " ";
            st.pop();
        }

    }
    else
    {
        std::cout << "Impossible"; // If I add return before this it does not give error in some cases.
    }
    cell* r = root;
    while(root)
    {
        root = root->parent;
        delete r;
        r = root;
    }
    return;
}

int main() {
    try
    {
    int n;
    cin >> n;
    unsigned short int i_start;
    unsigned short int j_start;
    unsigned short int i_end;
    unsigned short int j_end;
    cin >> i_start >> j_start >> i_end >> j_end;
    cout << "safe1\n";
    printShortestPath(n, i_start, j_start, i_end, j_end);
    }
    catch(...)
    {
        cout << "Not enough memory";
    }
    return 0;
}

When I give big input example
100
0 0 0 19
the output is
safe1
safe1.2
safe1.3
safe1.5

and the error shown is

solution: malloc.c:2394: sysmalloc: Assertion `(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)' failed.

GDB trace:
Reading symbols from solution...done.
[New LWP 14031]
Core was generated by `solution'.
Program terminated with signal SIGABRT, Aborted.
#0  0x00007f92ff084428 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54
#0  0x00007f92ff084428 in __GI_raise (sig=sig@entry=6)
    at ../sysdeps/unix/sysv/linux/raise.c:54
#1  0x00007f92ff08602a in __GI_abort () at abort.c:89
#2  0x00007f92ff0cc2e8 in __malloc_assert (
    assertion=assertion@entry=0x7f92ff1e0150 "(old_top == initial_top (av) && old_size == 0) || ((unsigned long) (old_size) >= MINSIZE && prev_inuse (old_top) && ((unsigned long) old_end & (pagesize - 1)) == 0)", 
    file=file@entry=0x7f92ff1dcb85 "malloc.c", line=line@entry=2394, 
    function=function@entry=0x7f92ff1e0998 <__func__.11509> "sysmalloc")
    at malloc.c:301
#3  0x00007f92ff0d0426 in sysmalloc (nb=nb@entry=64, 
    av=av@entry=0x7f92ff413b20 <main_arena>) at malloc.c:2391
#4  0x00007f92ff0d1743 in _int_malloc (
    av=av@entry=0x7f92ff413b20 <main_arena>, bytes=bytes@entry=48)
    at malloc.c:3827
#5  0x00007f92ff0d3184 in __GI___libc_malloc (bytes=48) at malloc.c:2913
#6  0x00007f92ff6c0498 in operator new(unsigned long) ()
   from /usr/lib/x86_64-linux-gnu/libstdc++.so.6
#7  0x0000000000401499 in printShortestPath (n=100, i_start=<optimized out>, 
    j_start=<optimized out>, i_end=0, j_end=19)
    at solution.cc:38
#8  0x0000000000401018 in main () at solution.cc:135

It is clear from the output that "safe1.4" is not printed and so the error seems to be at line with the comment "ERROR HERE"
PLEASE HELP.

  • `*(vis+i) = new bool` One per pointer and accessing out of bounds? – iBug Dec 16 '17 at 07:34
  • 2
    Here's some reasoning why [you shouldn't use `new` at all](https://stackoverflow.com/questions/46991224/are-there-any-valid-use-cases-to-use-new-and-delete-raw-pointers-or-c-style-arr) – user0042 Dec 16 '17 at 07:35
  • `#include ` -- Stop using this and use the proper C++ header files. – PaulMcKenzie Dec 16 '17 at 08:01
  • @PaulMcKenzie Why what's the problem with #include – Anurag Shah Dec 16 '17 at 08:12
  • @AnuragShah [Why should I not #include ](https://stackoverflow.com/questions/31816095/why-should-i-not-include-bits-stdc-h). Also `std::vector> vis(n+1, std::vector(n+1));` replaces both of those ugly loops and doesn't introduce memory leaks. – PaulMcKenzie Dec 16 '17 at 08:15
  • @AnuragShah -- Also, put the data you're using inside of the program. Don't post `cin` statements. You should have simply stated `printShortestPath(100, 0, 0,0, 19);`. No one is going to sit there and type in over and over again the input you're using every time the program is run. – PaulMcKenzie Dec 16 '17 at 08:28
  • @AnuragShah -- What compiler are you using? Didn't it give the warning that you're using an uninitialized variable `aa`? [See here](http://coliru.stacked-crooked.com/a/d980f1e92a71d757). Don't ignore the warnings -- fix the issues that the compiler is informing you of. You also forgot `#include `. – PaulMcKenzie Dec 16 '17 at 08:34
  • Actually I was coding online and your comment to use vector instead of manual memory worked perfectly.I am really thankful to you.Can you explain what is the second parameter you are passing in the constructor. – Anurag Shah Dec 16 '17 at 08:40
  • Yes it gave me warnings and I have fixed them now.Thanks. – Anurag Shah Dec 16 '17 at 08:40
  • @AnuragShah -- The second parameter is the initialization of the inner vector. If you really wanted a 2d-array of `bool` using `new`, here is a [much cleaner way of doing so](https://stackoverflow.com/questions/21943621/how-to-create-a-contiguous-2d-array-in-c/21944048#21944048). Just replace `double` with `bool`. – PaulMcKenzie Dec 16 '17 at 08:41

1 Answers1

1

I did not read all the code in detail however the line:

for(unsigned short int i = 0; i < n+1; i++) *(vis+i) = new bool;

looks suspect. If this is what I think it is, it is an attempt to assign bool pointers to each element in an array of bool pointers of size n+1.

However adding an int to a pointer is very likely not going to give the intended result. See http://www.cplusplus.com/doc/tutorial/pointers/

Might try:

bool* cur_vis_ptr =*vis;
for(unsigned short int i = 0; i < n+1; i++){
   cur_vis_ptr=new bool;
   cur_vis_ptr++;
}

Or perhaps better the simple way:

for(unsigned short int i = 0; i < n+1; i++)
   vis[i]=new bool;